From 7e90d705fc9f8c5e3a1549281dce0654d049243b Mon Sep 17 00:00:00 2001 From: Steve French Date: Tue, 8 Feb 2011 23:52:32 +0000 Subject: [CIFS] Do not send SMBEcho requests on new sockets until SMBNegotiate In order to determine whether an SMBEcho request can be sent we need to know that the socket is established (server tcpStatus == CifsGood) AND that an SMB NegotiateProtocol has been sent (server maxBuf != 0). Without the second check we can send an Echo request during reconnection before the server can accept it. CC: JG Reviewed-by: Jeff Layton Signed-off-by: Steve French --- fs/cifs/cifsglob.h | 2 ++ fs/cifs/connect.c | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) (limited to 'fs/cifs') diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h index edd5b29b53c..1ab33eb71d9 100644 --- a/fs/cifs/cifsglob.h +++ b/fs/cifs/cifsglob.h @@ -188,6 +188,8 @@ struct TCP_Server_Info { /* multiplexed reads or writes */ unsigned int maxBuf; /* maxBuf specifies the maximum */ /* message size the server can send or receive for non-raw SMBs */ + /* maxBuf is returned by SMB NegotiateProtocol so maxBuf is only 0 */ + /* when socket is setup (and during reconnect) before NegProt sent */ unsigned int max_rw; /* maxRw specifies the maximum */ /* message size the server can send or receive for */ /* SMB_COM_WRITE_RAW or SMB_COM_READ_RAW. */ diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c index 257b6d895e2..10011e99b34 100644 --- a/fs/cifs/connect.c +++ b/fs/cifs/connect.c @@ -341,7 +341,7 @@ cifs_echo_request(struct work_struct *work) * We cannot send an echo until the NEGOTIATE_PROTOCOL request is done. * Also, no need to ping if we got a response recently */ - if (server->tcpStatus != CifsGood || + if ((server->tcpStatus != CifsGood) || (server->maxBuf == 0) || time_before(jiffies, server->lstrp + SMB_ECHO_INTERVAL - HZ)) goto requeue_echo; -- cgit v1.2.3-70-g09d2 From 195291e68c2ad59a046fc56d32bf59635b100e5c Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Wed, 9 Feb 2011 12:01:42 -0500 Subject: cifs: clean up checks in cifs_echo_request Follow-on patch to 7e90d705 which is already in Steve's tree... The check for tcpStatus == CifsGood is not meaningful since it doesn't indicate whether the NEGOTIATE request has been done. Also, clarify why we're checking for maxBuf == 0. Signed-off-by: Jeff Layton Signed-off-by: Steve French --- fs/cifs/connect.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'fs/cifs') diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c index 10011e99b34..161f24ca4f6 100644 --- a/fs/cifs/connect.c +++ b/fs/cifs/connect.c @@ -338,10 +338,11 @@ cifs_echo_request(struct work_struct *work) struct TCP_Server_Info, echo.work); /* - * We cannot send an echo until the NEGOTIATE_PROTOCOL request is done. - * Also, no need to ping if we got a response recently + * We cannot send an echo until the NEGOTIATE_PROTOCOL request is + * done, which is indicated by maxBuf != 0. Also, no need to ping if + * we got a response recently */ - if ((server->tcpStatus != CifsGood) || (server->maxBuf == 0) || + if (server->maxBuf == 0 || time_before(jiffies, server->lstrp + SMB_ECHO_INTERVAL - HZ)) goto requeue_echo; -- cgit v1.2.3-70-g09d2 From 71823baff1978be892e7a36eddf6170e1cc6650d Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Thu, 10 Feb 2011 08:03:50 -0500 Subject: cifs: don't always drop malformed replies on the floor (try #3) 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 Signed-off-by: Steve French --- fs/cifs/cifsglob.h | 2 +- fs/cifs/connect.c | 30 ++++++++++++++++++++++++------ fs/cifs/transport.c | 3 +++ 3 files changed, 28 insertions(+), 7 deletions(-) (limited to 'fs/cifs') diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h index 1ab33eb71d9..17afb0fbcae 100644 --- a/fs/cifs/cifsglob.h +++ b/fs/cifs/cifsglob.h @@ -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 */ diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c index 161f24ca4f6..8d6c17ab593 100644 --- a/fs/cifs/connect.c +++ b/fs/cifs/connect.c @@ -586,11 +586,20 @@ incomplete_rcv: 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; @@ -602,7 +611,8 @@ incomplete_rcv: 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) { @@ -637,7 +647,12 @@ incomplete_rcv: 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 @@ -658,6 +673,9 @@ multi_t2_fnd: 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! " diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c index fbc5aace54b..46d8756f2b2 100644 --- a/fs/cifs/transport.c +++ b/fs/cifs/transport.c @@ -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); -- cgit v1.2.3-70-g09d2