Skip to content

Commit

Permalink
cifs: don't always drop malformed replies on the floor (try #3)
Browse files Browse the repository at this point in the history
Slight revision to this patch...use min_t() instead of conditional
assignment. Also, remove the FIXME comment and replace it with the
explanation that Steve gave earlier.

After receiving a packet, we currently check the header. If it's no
good, then we toss it out and continue the loop, leaving the caller
waiting on that response.

In cases where the packet has length inconsistencies, but the MID is
valid, this leads to unneeded delays. That's especially problematic now
that the client waits indefinitely for responses.

Instead, don't immediately discard the packet if checkSMB fails. Try to
find a matching mid_q_entry, mark it as having a malformed response and
issue the callback.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
Signed-off-by: Steve French <sfrench@us.ibm.com>
  • Loading branch information
Jeff Layton authored and Steve French committed Feb 11, 2011
1 parent 195291e commit 71823ba
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 7 deletions.
2 changes: 1 addition & 1 deletion fs/cifs/cifsglob.h
Original file line number Diff line number Diff line change
Expand Up @@ -654,7 +654,7 @@ static inline void free_dfs_info_array(struct dfs_info3_param *param,
#define MID_REQUEST_SUBMITTED 2
#define MID_RESPONSE_RECEIVED 4
#define MID_RETRY_NEEDED 8 /* session closed while this request out */
#define MID_NO_RESP_NEEDED 0x10
#define MID_RESPONSE_MALFORMED 0x10

/* Types of response buffer returned from SendReceive2 */
#define CIFS_NO_BUFFER 0 /* Response buffer not returned */
Expand Down
30 changes: 24 additions & 6 deletions fs/cifs/connect.c
Original file line number Diff line number Diff line change
Expand Up @@ -586,11 +586,20 @@ cifs_demultiplex_thread(struct TCP_Server_Info *server)
total_read += 4; /* account for rfc1002 hdr */

dump_smb(smb_buffer, total_read);
if (checkSMB(smb_buffer, smb_buffer->Mid, total_read)) {

/*
* We know that we received enough to get to the MID as we
* checked the pdu_length earlier. Now check to see
* if the rest of the header is OK. We borrow the length
* var for the rest of the loop to avoid a new stack var.
*
* 48 bytes is enough to display the header and a little bit
* into the payload for debugging purposes.
*/
length = checkSMB(smb_buffer, smb_buffer->Mid, total_read);
if (length != 0)
cifs_dump_mem("Bad SMB: ", smb_buffer,
total_read < 48 ? total_read : 48);
continue;
}
min_t(unsigned int, total_read, 48));

mid_entry = NULL;
server->lstrp = jiffies;
Expand All @@ -602,7 +611,8 @@ cifs_demultiplex_thread(struct TCP_Server_Info *server)
if ((mid_entry->mid == smb_buffer->Mid) &&
(mid_entry->midState == MID_REQUEST_SUBMITTED) &&
(mid_entry->command == smb_buffer->Command)) {
if (check2ndT2(smb_buffer,server->maxBuf) > 0) {
if (length == 0 &&
check2ndT2(smb_buffer, server->maxBuf) > 0) {
/* We have a multipart transact2 resp */
isMultiRsp = true;
if (mid_entry->resp_buf) {
Expand Down Expand Up @@ -637,7 +647,12 @@ cifs_demultiplex_thread(struct TCP_Server_Info *server)
mid_entry->resp_buf = smb_buffer;
mid_entry->largeBuf = isLargeBuf;
multi_t2_fnd:
mid_entry->midState = MID_RESPONSE_RECEIVED;
if (length == 0)
mid_entry->midState =
MID_RESPONSE_RECEIVED;
else
mid_entry->midState =
MID_RESPONSE_MALFORMED;
#ifdef CONFIG_CIFS_STATS2
mid_entry->when_received = jiffies;
#endif
Expand All @@ -658,6 +673,9 @@ cifs_demultiplex_thread(struct TCP_Server_Info *server)
else
smallbuf = NULL;
}
} else if (length != 0) {
/* response sanity checks failed */
continue;
} else if (!is_valid_oplock_break(smb_buffer, server) &&
!isMultiRsp) {
cERROR(1, "No task to wake, unknown frame received! "
Expand Down
3 changes: 3 additions & 0 deletions fs/cifs/transport.c
Original file line number Diff line number Diff line change
Expand Up @@ -457,6 +457,9 @@ sync_mid_result(struct mid_q_entry *mid, struct TCP_Server_Info *server)
case MID_RETRY_NEEDED:
rc = -EAGAIN;
break;
case MID_RESPONSE_MALFORMED:
rc = -EIO;
break;
default:
cERROR(1, "%s: invalid mid state mid=%d state=%d", __func__,
mid->mid, mid->midState);
Expand Down

0 comments on commit 71823ba

Please sign in to comment.