From c4d3396b261473ded6f370edd1e79ba34e089d7e Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Tue, 26 Jul 2011 12:20:18 -0400 Subject: cifs: advertise the right receive buffer size to the server Currently, we mirror the same size back to the server that it sends us. That makes little sense. Instead we should be sending the server the maximum buffer size that we can handle -- CIFSMaxBufSize minus the 4 byte RFC1001 header. Signed-off-by: Jeff Layton Signed-off-by: Steve French --- fs/cifs/sess.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'fs') diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c index d3e619692ee..243d5872051 100644 --- a/fs/cifs/sess.c +++ b/fs/cifs/sess.c @@ -124,7 +124,8 @@ static __u32 cifs_ssetup_hdr(struct cifs_ses *ses, SESSION_SETUP_ANDX *pSMB) /* that we use in next few lines */ /* Note that header is initialized to zero in header_assemble */ pSMB->req.AndXCommand = 0xFF; - pSMB->req.MaxBufferSize = cpu_to_le16(ses->server->maxBuf); + pSMB->req.MaxBufferSize = cpu_to_le16(min_t(u32, CIFSMaxBufSize - 4, + USHRT_MAX)); pSMB->req.MaxMpxCount = cpu_to_le16(ses->server->maxReq); pSMB->req.VcNumber = get_next_vcnum(ses); -- cgit v1.2.3 From 1f1cff0be05f59d5939edf28ff5ca0c6fd0a8e1c Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Tue, 26 Jul 2011 12:20:18 -0400 Subject: cifs: trivial: goto out here is unnecessary ...and remove some obsolete comments. Signed-off-by: Jeff Layton Signed-off-by: Steve French --- fs/cifs/connect.c | 6 ------ 1 file changed, 6 deletions(-) (limited to 'fs') diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c index e66297bad41..50b3523912f 100644 --- a/fs/cifs/connect.c +++ b/fs/cifs/connect.c @@ -3193,15 +3193,9 @@ mount_fail_check: else cifs_put_tcp_session(srvTcp); bdi_destroy(&cifs_sb->bdi); - goto out; } - /* volume_info->password is freed above when existing session found - (in which case it is not needed anymore) but when new sesion is created - the password ptr is put in the new session structure (in which case the - password will be freed at unmount time) */ out: - /* zero out password before freeing */ FreeXid(xid); return rc; } -- cgit v1.2.3 From 998d6fcb24d25b7889ec39118cf98d5089ac4c11 Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Tue, 26 Jul 2011 12:21:17 -0400 Subject: cifs: don't start signing too early Sniffing traffic on the wire shows that windows clients send a zeroed out signature field in a NEGOTIATE request, and send "BSRSPYL" in the signature field during SESSION_SETUP. Make the cifs client behave the same way. It doesn't seem to make much difference in any server that I've tested against, but it's probably best to follow windows behavior as closely as possible here. Signed-off-by: Jeff Layton Reviewed-by: Shirish Pargaonkar Signed-off-by: Steve French --- fs/cifs/cifsencrypt.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) (limited to 'fs') diff --git a/fs/cifs/cifsencrypt.c b/fs/cifs/cifsencrypt.c index 259991bd211..e76bfeb6826 100644 --- a/fs/cifs/cifsencrypt.c +++ b/fs/cifs/cifsencrypt.c @@ -87,9 +87,15 @@ int cifs_sign_smb(struct smb_hdr *cifs_pdu, struct TCP_Server_Info *server, if ((cifs_pdu == NULL) || (server == NULL)) return -EINVAL; - if ((cifs_pdu->Flags2 & SMBFLG2_SECURITY_SIGNATURE) == 0) + if (!(cifs_pdu->Flags2 & SMBFLG2_SECURITY_SIGNATURE) || + server->tcpStatus == CifsNeedNegotiate) return rc; + if (!server->session_estab) { + strncpy(cifs_pdu->Signature.SecuritySignature, "BSRSPYL", 8); + return rc; + } + cifs_pdu->Signature.Sequence.SequenceNumber = cpu_to_le32(server->sequence_number); cifs_pdu->Signature.Sequence.Reserved = 0; @@ -178,9 +184,15 @@ int cifs_sign_smb2(struct kvec *iov, int n_vec, struct TCP_Server_Info *server, if ((cifs_pdu == NULL) || (server == NULL)) return -EINVAL; - if ((cifs_pdu->Flags2 & SMBFLG2_SECURITY_SIGNATURE) == 0) + if (!(cifs_pdu->Flags2 & SMBFLG2_SECURITY_SIGNATURE) || + server->tcpStatus == CifsNeedNegotiate) return rc; + if (!server->session_estab) { + strncpy(cifs_pdu->Signature.SecuritySignature, "BSRSPYL", 8); + return rc; + } + cifs_pdu->Signature.Sequence.SequenceNumber = cpu_to_le32(server->sequence_number); cifs_pdu->Signature.Sequence.Reserved = 0; -- cgit v1.2.3 From 91d065c47317cd5f6577fa077cca3383c8d9243d Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Tue, 26 Jul 2011 18:23:47 -0400 Subject: cifs: fix name parsing in CIFSSMBQAllEAs The code that matches EA names in CIFSSMBQAllEAs is incorrect. It uses strncmp to do the comparison with the length limited to the name_len sent in the response. Problem: Suppose we're looking for an attribute named "foobar" and have an attribute before it in the EA list named "foo". The comparison will succeed since we're only looking at the first 3 characters. Fix this by also comparing the length of the provided ea_name with the name_len in the response. If they're not equal then it shouldn't match. Reported-by: Jian Li Signed-off-by: Jeff Layton Reviewed-by: Pavel Shilovsky Signed-off-by: Steve French --- fs/cifs/cifssmb.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'fs') diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c index 1a9fe7f816d..0580da1cf34 100644 --- a/fs/cifs/cifssmb.c +++ b/fs/cifs/cifssmb.c @@ -5720,6 +5720,7 @@ CIFSSMBQAllEAs(const int xid, struct cifs_tcon *tcon, char *temp_ptr; char *end_of_smb; __u16 params, byte_count, data_offset; + unsigned int ea_name_len; cFYI(1, "In Query All EAs path %s", searchName); QAllEAsRetry: @@ -5814,6 +5815,10 @@ QAllEAsRetry: list_len -= 4; temp_fea = ea_response_data->list; temp_ptr = (char *)temp_fea; + + if (ea_name) + ea_name_len = strlen(ea_name); + while (list_len > 0) { unsigned int name_len; __u16 value_len; @@ -5837,7 +5842,8 @@ QAllEAsRetry: } if (ea_name) { - if (strncmp(ea_name, temp_ptr, name_len) == 0) { + if (ea_name_len == name_len && + strncmp(ea_name, temp_ptr, name_len) == 0) { temp_ptr += name_len + 1; rc = value_len; if (buf_size == 0) -- cgit v1.2.3 From 5980fc966bb347801f3fcc39a2ef2a1e14fbf8cb Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Thu, 28 Jul 2011 12:48:26 -0400 Subject: cifs: fix compiler warning in CIFSSMBQAllEAs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The recent fix to the above function causes this compiler warning to pop on some gcc versions: CC [M] fs/cifs/cifssmb.o fs/cifs/cifssmb.c: In function ‘CIFSSMBQAllEAs’: fs/cifs/cifssmb.c:5708: warning: ‘ea_name_len’ may be used uninitialized in this function Signed-off-by: Jeff Layton Signed-off-by: Steve French --- fs/cifs/cifssmb.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) (limited to 'fs') diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c index 0580da1cf34..187afe38dac 100644 --- a/fs/cifs/cifssmb.c +++ b/fs/cifs/cifssmb.c @@ -5720,7 +5720,7 @@ CIFSSMBQAllEAs(const int xid, struct cifs_tcon *tcon, char *temp_ptr; char *end_of_smb; __u16 params, byte_count, data_offset; - unsigned int ea_name_len; + unsigned int ea_name_len = ea_name ? strlen(ea_name) : 0; cFYI(1, "In Query All EAs path %s", searchName); QAllEAsRetry: @@ -5815,10 +5815,6 @@ QAllEAsRetry: list_len -= 4; temp_fea = ea_response_data->list; temp_ptr = (char *)temp_fea; - - if (ea_name) - ea_name_len = strlen(ea_name); - while (list_len > 0) { unsigned int name_len; __u16 value_len; -- cgit v1.2.3 From ad635942c869ad8fc9af270d4998c42b4e978b32 Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Tue, 26 Jul 2011 12:20:17 -0400 Subject: cifs: simplify refcounting for oplock breaks Currently, we take a sb->s_active reference and a cifsFileInfo reference when an oplock break workqueue job is queued. This is unnecessary and more complicated than it needs to be. Also as Al points out, deactivate_super has non-trivial locking implications so it's best to avoid that if we can. Instead, just cancel any pending oplock breaks for this filehandle synchronously in cifsFileInfo_put after taking it off the lists. That should ensure that this job doesn't outlive the structures it depends on. Reported-by: Al Viro Signed-off-by: Jeff Layton Signed-off-by: Steve French --- fs/cifs/cifsfs.c | 18 ------------------ fs/cifs/cifsfs.h | 4 ---- fs/cifs/cifsglob.h | 2 -- fs/cifs/file.c | 27 ++------------------------- fs/cifs/misc.c | 11 ++--------- 5 files changed, 4 insertions(+), 58 deletions(-) (limited to 'fs') diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c index 86551747096..212e5629cc1 100644 --- a/fs/cifs/cifsfs.c +++ b/fs/cifs/cifsfs.c @@ -86,24 +86,6 @@ extern mempool_t *cifs_sm_req_poolp; extern mempool_t *cifs_req_poolp; extern mempool_t *cifs_mid_poolp; -void -cifs_sb_active(struct super_block *sb) -{ - struct cifs_sb_info *server = CIFS_SB(sb); - - if (atomic_inc_return(&server->active) == 1) - atomic_inc(&sb->s_active); -} - -void -cifs_sb_deactive(struct super_block *sb) -{ - struct cifs_sb_info *server = CIFS_SB(sb); - - if (atomic_dec_and_test(&server->active)) - deactivate_super(sb); -} - static int cifs_read_super(struct super_block *sb) { diff --git a/fs/cifs/cifsfs.h b/fs/cifs/cifsfs.h index fbd050c8d52..cb71dc1f94d 100644 --- a/fs/cifs/cifsfs.h +++ b/fs/cifs/cifsfs.h @@ -41,10 +41,6 @@ extern struct file_system_type cifs_fs_type; extern const struct address_space_operations cifs_addr_ops; extern const struct address_space_operations cifs_addr_ops_smallbuf; -/* Functions related to super block operations */ -extern void cifs_sb_active(struct super_block *sb); -extern void cifs_sb_deactive(struct super_block *sb); - /* Functions related to inodes */ extern const struct inode_operations cifs_dir_inode_ops; extern struct inode *cifs_root_iget(struct super_block *); diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h index 1fcf4e5b311..38ce6d44b14 100644 --- a/fs/cifs/cifsglob.h +++ b/fs/cifs/cifsglob.h @@ -942,8 +942,6 @@ GLOBAL_EXTERN spinlock_t siduidlock; GLOBAL_EXTERN spinlock_t sidgidlock; void cifs_oplock_break(struct work_struct *work); -void cifs_oplock_break_get(struct cifsFileInfo *cfile); -void cifs_oplock_break_put(struct cifsFileInfo *cfile); extern const struct slow_work_ops cifs_oplock_break_ops; diff --git a/fs/cifs/file.c b/fs/cifs/file.c index 378acdafa35..9f41a10523a 100644 --- a/fs/cifs/file.c +++ b/fs/cifs/file.c @@ -314,6 +314,8 @@ void cifsFileInfo_put(struct cifsFileInfo *cifs_file) } spin_unlock(&cifs_file_list_lock); + cancel_work_sync(&cifs_file->oplock_break); + if (!tcon->need_reconnect && !cifs_file->invalidHandle) { int xid, rc; @@ -2418,31 +2420,6 @@ void cifs_oplock_break(struct work_struct *work) cinode->clientCanCacheRead ? 1 : 0); cFYI(1, "Oplock release rc = %d", rc); } - - /* - * We might have kicked in before is_valid_oplock_break() - * finished grabbing reference for us. Make sure it's done by - * waiting for cifs_file_list_lock. - */ - spin_lock(&cifs_file_list_lock); - spin_unlock(&cifs_file_list_lock); - - cifs_oplock_break_put(cfile); -} - -/* must be called while holding cifs_file_list_lock */ -void cifs_oplock_break_get(struct cifsFileInfo *cfile) -{ - cifs_sb_active(cfile->dentry->d_sb); - cifsFileInfo_get(cfile); -} - -void cifs_oplock_break_put(struct cifsFileInfo *cfile) -{ - struct super_block *sb = cfile->dentry->d_sb; - - cifsFileInfo_put(cfile); - cifs_sb_deactive(sb); } const struct address_space_operations cifs_addr_ops = { diff --git a/fs/cifs/misc.c b/fs/cifs/misc.c index 03a1f491d39..7c169339259 100644 --- a/fs/cifs/misc.c +++ b/fs/cifs/misc.c @@ -585,15 +585,8 @@ is_valid_oplock_break(struct smb_hdr *buf, struct TCP_Server_Info *srv) cifs_set_oplock_level(pCifsInode, pSMB->OplockLevel ? OPLOCK_READ : 0); - /* - * cifs_oplock_break_put() can't be called - * from here. Get reference after queueing - * succeeded. cifs_oplock_break() will - * synchronize using cifs_file_list_lock. - */ - if (queue_work(system_nrt_wq, - &netfile->oplock_break)) - cifs_oplock_break_get(netfile); + queue_work(system_nrt_wq, + &netfile->oplock_break); netfile->oplock_break_cancelled = false; spin_unlock(&cifs_file_list_lock); -- cgit v1.2.3 From c4a5534a1b61cdffaa83187efe63712f75544726 Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Thu, 28 Jul 2011 12:40:36 -0400 Subject: cifs: remove unneeded variable initialization in cifs_reconnect_tcon Reported-and-acked-by: David Howells Signed-off-by: Jeff Layton Signed-off-by: Steve French --- fs/cifs/cifssmb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'fs') diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c index 187afe38dac..aac37d99a48 100644 --- a/fs/cifs/cifssmb.c +++ b/fs/cifs/cifssmb.c @@ -107,7 +107,7 @@ static void mark_open_files_invalid(struct cifs_tcon *pTcon) static int cifs_reconnect_tcon(struct cifs_tcon *tcon, int smb_command) { - int rc = 0; + int rc; struct cifs_ses *ses; struct TCP_Server_Info *server; struct nls_table *nls_codepage; -- cgit v1.2.3 From 3d9c2472a53ee1d26de9803899037aeeb44ccef1 Mon Sep 17 00:00:00 2001 From: Pavel Shilovsky Date: Mon, 1 Aug 2011 13:19:40 +0400 Subject: CIFS: Move buffer allocation to a separate function Reviewed-and-Tested-by: Jeff Layton Signed-off-by: Pavel Shilovsky Signed-off-by: Steve French --- fs/cifs/connect.c | 92 +++++++++++++++++++++++++++++++++---------------------- 1 file changed, 55 insertions(+), 37 deletions(-) (limited to 'fs') diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c index 50b3523912f..217d3658718 100644 --- a/fs/cifs/connect.c +++ b/fs/cifs/connect.c @@ -319,15 +319,53 @@ requeue_echo: queue_delayed_work(system_nrt_wq, &server->echo, SMB_ECHO_INTERVAL); } +static bool +allocate_buffers(char **bigbuf, char **smallbuf, unsigned int size, + bool is_large_buf) +{ + char *bbuf = *bigbuf, *sbuf = *smallbuf; + + if (bbuf == NULL) { + bbuf = (char *)cifs_buf_get(); + if (!bbuf) { + cERROR(1, "No memory for large SMB response"); + msleep(3000); + /* retry will check if exiting */ + return false; + } + } else if (is_large_buf) { + /* we are reusing a dirty large buf, clear its start */ + memset(bbuf, 0, size); + } + + if (sbuf == NULL) { + sbuf = (char *)cifs_small_buf_get(); + if (!sbuf) { + cERROR(1, "No memory for SMB response"); + msleep(1000); + /* retry will check if exiting */ + return false; + } + /* beginning of smb buffer is cleared in our buf_get */ + } else { + /* if existing small buf clear beginning */ + memset(sbuf, 0, size); + } + + *bigbuf = bbuf; + *smallbuf = sbuf; + + return true; +} + static int cifs_demultiplex_thread(void *p) { int length; struct TCP_Server_Info *server = p; unsigned int pdu_length, total_read; + char *buf = NULL, *bigbuf = NULL, *smallbuf = NULL; struct smb_hdr *smb_buffer = NULL; - struct smb_hdr *bigbuf = NULL; - struct smb_hdr *smallbuf = NULL; struct msghdr smb_msg; struct kvec iov; struct socket *csocket = server->ssocket; @@ -351,35 +389,16 @@ cifs_demultiplex_thread(void *p) while (server->tcpStatus != CifsExiting) { if (try_to_freeze()) continue; - if (bigbuf == NULL) { - bigbuf = cifs_buf_get(); - if (!bigbuf) { - cERROR(1, "No memory for large SMB response"); - msleep(3000); - /* retry will check if exiting */ - continue; - } - } else if (isLargeBuf) { - /* we are reusing a dirty large buf, clear its start */ - memset(bigbuf, 0, sizeof(struct smb_hdr)); - } - if (smallbuf == NULL) { - smallbuf = cifs_small_buf_get(); - if (!smallbuf) { - cERROR(1, "No memory for SMB response"); - msleep(1000); - /* retry will check if exiting */ - continue; - } - /* beginning of smb buffer is cleared in our buf_get */ - } else /* if existing small buf clear beginning */ - memset(smallbuf, 0, sizeof(struct smb_hdr)); + if (!allocate_buffers(&bigbuf, &smallbuf, + sizeof(struct smb_hdr), isLargeBuf)) + continue; isLargeBuf = false; isMultiRsp = false; - smb_buffer = smallbuf; - iov.iov_base = smb_buffer; + smb_buffer = (struct smb_hdr *)smallbuf; + buf = smallbuf; + iov.iov_base = buf; iov.iov_len = 4; smb_msg.msg_control = NULL; smb_msg.msg_controllen = 0; @@ -417,8 +436,7 @@ incomplete_rcv: allowing socket to clear and app threads to set tcpStatus CifsNeedReconnect if server hung */ if (pdu_length < 4) { - iov.iov_base = (4 - pdu_length) + - (char *)smb_buffer; + iov.iov_base = (4 - pdu_length) + buf; iov.iov_len = pdu_length; smb_msg.msg_control = NULL; smb_msg.msg_controllen = 0; @@ -446,7 +464,7 @@ incomplete_rcv: /* the first byte big endian of the length field, is actually not part of the length but the type with the most common, zero, as regular data */ - temp = *((char *) smb_buffer); + temp = *buf; /* Note that FC 1001 length is big endian on the wire, but we convert it here so it is always manipulated @@ -480,8 +498,7 @@ incomplete_rcv: continue; } else if (temp != (char) 0) { cERROR(1, "Unknown RFC 1002 frame"); - cifs_dump_mem(" Received Data: ", (char *)smb_buffer, - length); + cifs_dump_mem(" Received Data: ", buf, length); cifs_reconnect(server); csocket = server->ssocket; continue; @@ -504,10 +521,11 @@ incomplete_rcv: if (pdu_length > MAX_CIFS_SMALL_BUFFER_SIZE - 4) { isLargeBuf = true; memcpy(bigbuf, smallbuf, 4); - smb_buffer = bigbuf; + smb_buffer = (struct smb_hdr *)bigbuf; + buf = bigbuf; } length = 0; - iov.iov_base = 4 + (char *)smb_buffer; + iov.iov_base = 4 + buf; iov.iov_len = pdu_length; for (total_read = 0; total_read < pdu_length; total_read += length) { @@ -562,8 +580,8 @@ incomplete_rcv: */ length = checkSMB(smb_buffer, smb_buffer->Mid, total_read); if (length != 0) - cifs_dump_mem("Bad SMB: ", smb_buffer, - min_t(unsigned int, total_read, 48)); + cifs_dump_mem("Bad SMB: ", buf, + min_t(unsigned int, total_read, 48)); mid_entry = NULL; server->lstrp = jiffies; @@ -648,7 +666,7 @@ multi_t2_fnd: !isMultiRsp) { cERROR(1, "No task to wake, unknown frame received! " "NumMids %d", atomic_read(&midCount)); - cifs_dump_mem("Received Data is: ", (char *)smb_buffer, + cifs_dump_mem("Received Data is: ", buf, sizeof(struct smb_hdr)); #ifdef CONFIG_CIFS_DEBUG2 cifs_dump_detail(smb_buffer); -- cgit v1.2.3 From e7015fb1c508fe9b8c97707755ce08f5ace0afb9 Mon Sep 17 00:00:00 2001 From: Pavel Shilovsky Date: Mon, 1 Aug 2011 13:19:41 +0400 Subject: CIFS: Simplify socket reading in demultiplex thread Move reading to separate function and remove csocket variable. Also change semantic in a little: goto incomplete_rcv only when we get -EAGAIN (or a familiar error) while reading rfc1002 header. In this case we don't check for echo timeout when we don't get whole header at once, as it was before. Reviewed-and-Tested-by: Jeff Layton Signed-off-by: Pavel Shilovsky Signed-off-by: Steve French --- fs/cifs/connect.c | 154 +++++++++++++++++++++++++----------------------------- 1 file changed, 71 insertions(+), 83 deletions(-) (limited to 'fs') diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c index 217d3658718..a2e9fcf4a99 100644 --- a/fs/cifs/connect.c +++ b/fs/cifs/connect.c @@ -358,6 +358,64 @@ allocate_buffers(char **bigbuf, char **smallbuf, unsigned int size, return true; } +static int +read_from_socket(struct TCP_Server_Info *server, struct msghdr *smb_msg, + struct kvec *iov, unsigned int to_read, + unsigned int *ptotal_read, bool is_header_read) +{ + int length, rc = 0; + unsigned int total_read; + char *buf = iov->iov_base; + + for (total_read = 0; total_read < to_read; total_read += length) { + length = kernel_recvmsg(server->ssocket, smb_msg, iov, 1, + to_read - total_read, 0); + if (server->tcpStatus == CifsExiting) { + /* then will exit */ + rc = 2; + break; + } else if (server->tcpStatus == CifsNeedReconnect) { + cifs_reconnect(server); + /* Reconnect wakes up rspns q */ + /* Now we will reread sock */ + rc = 1; + break; + } else if (length == -ERESTARTSYS || + length == -EAGAIN || + length == -EINTR) { + /* + * Minimum sleep to prevent looping, allowing socket + * to clear and app threads to set tcpStatus + * CifsNeedReconnect if server hung. + */ + usleep_range(1000, 2000); + length = 0; + if (!is_header_read) + continue; + /* Special handling for header read */ + if (total_read) { + iov->iov_base = (to_read - total_read) + + buf; + iov->iov_len = to_read - total_read; + smb_msg->msg_control = NULL; + smb_msg->msg_controllen = 0; + rc = 3; + } else + rc = 1; + break; + } else if (length <= 0) { + cERROR(1, "Received no data, expecting %d", + to_read - total_read); + cifs_reconnect(server); + rc = 1; + break; + } + } + + *ptotal_read = total_read; + return rc; +} + static int cifs_demultiplex_thread(void *p) { @@ -368,14 +426,13 @@ cifs_demultiplex_thread(void *p) struct smb_hdr *smb_buffer = NULL; struct msghdr smb_msg; struct kvec iov; - struct socket *csocket = server->ssocket; struct list_head *tmp, *tmp2; struct task_struct *task_to_wake = NULL; struct mid_q_entry *mid_entry; char temp; bool isLargeBuf = false; bool isMultiRsp; - int reconnect; + int rc; current->flags |= PF_MEMALLOC; cFYI(1, "Demultiplex PID: %d", task_pid_nr(current)); @@ -412,51 +469,18 @@ incomplete_rcv: "Reconnecting...", server->hostname, (echo_retries * SMB_ECHO_INTERVAL / HZ)); cifs_reconnect(server); - csocket = server->ssocket; wake_up(&server->response_q); continue; } - length = - kernel_recvmsg(csocket, &smb_msg, - &iov, 1, pdu_length, 0 /* BB other flags? */); - - if (server->tcpStatus == CifsExiting) { + rc = read_from_socket(server, &smb_msg, &iov, pdu_length, + &total_read, true /* header read */); + if (rc == 3) + goto incomplete_rcv; + else if (rc == 2) break; - } else if (server->tcpStatus == CifsNeedReconnect) { - cFYI(1, "Reconnect after server stopped responding"); - cifs_reconnect(server); - cFYI(1, "call to reconnect done"); - csocket = server->ssocket; - continue; - } else if (length == -ERESTARTSYS || - length == -EAGAIN || - length == -EINTR) { - msleep(1); /* minimum sleep to prevent looping - allowing socket to clear and app threads to set - tcpStatus CifsNeedReconnect if server hung */ - if (pdu_length < 4) { - iov.iov_base = (4 - pdu_length) + buf; - iov.iov_len = pdu_length; - smb_msg.msg_control = NULL; - smb_msg.msg_controllen = 0; - goto incomplete_rcv; - } else - continue; - } else if (length <= 0) { - cFYI(1, "Reconnect after unexpected peek error %d", - length); - cifs_reconnect(server); - csocket = server->ssocket; - wake_up(&server->response_q); + else if (rc == 1) continue; - } else if (length < pdu_length) { - cFYI(1, "requested %d bytes but only got %d bytes", - pdu_length, length); - pdu_length -= length; - msleep(1); - goto incomplete_rcv; - } /* The right amount was read from socket - 4 bytes */ /* so we can now interpret the length field */ @@ -493,14 +517,12 @@ incomplete_rcv: cifs_set_port((struct sockaddr *) &server->dstaddr, CIFS_PORT); cifs_reconnect(server); - csocket = server->ssocket; wake_up(&server->response_q); continue; } else if (temp != (char) 0) { cERROR(1, "Unknown RFC 1002 frame"); cifs_dump_mem(" Received Data: ", buf, length); cifs_reconnect(server); - csocket = server->ssocket; continue; } @@ -510,59 +532,25 @@ incomplete_rcv: cERROR(1, "Invalid size SMB length %d pdu_length %d", length, pdu_length+4); cifs_reconnect(server); - csocket = server->ssocket; wake_up(&server->response_q); continue; } /* else length ok */ - reconnect = 0; - if (pdu_length > MAX_CIFS_SMALL_BUFFER_SIZE - 4) { isLargeBuf = true; memcpy(bigbuf, smallbuf, 4); smb_buffer = (struct smb_hdr *)bigbuf; buf = bigbuf; } - length = 0; + iov.iov_base = 4 + buf; iov.iov_len = pdu_length; - for (total_read = 0; total_read < pdu_length; - total_read += length) { - length = kernel_recvmsg(csocket, &smb_msg, &iov, 1, - pdu_length - total_read, 0); - if (server->tcpStatus == CifsExiting) { - /* then will exit */ - reconnect = 2; - break; - } else if (server->tcpStatus == CifsNeedReconnect) { - cifs_reconnect(server); - csocket = server->ssocket; - /* Reconnect wakes up rspns q */ - /* Now we will reread sock */ - reconnect = 1; - break; - } else if (length == -ERESTARTSYS || - length == -EAGAIN || - length == -EINTR) { - msleep(1); /* minimum sleep to prevent looping, - allowing socket to clear and app - threads to set tcpStatus - CifsNeedReconnect if server hung*/ - length = 0; - continue; - } else if (length <= 0) { - cERROR(1, "Received no data, expecting %d", - pdu_length - total_read); - cifs_reconnect(server); - csocket = server->ssocket; - reconnect = 1; - break; - } - } - if (reconnect == 2) + rc = read_from_socket(server, &smb_msg, &iov, pdu_length, + &total_read, false); + if (rc == 2) break; - else if (reconnect == 1) + else if (rc == 1) continue; total_read += 4; /* account for rfc1002 hdr */ @@ -705,7 +693,7 @@ multi_t2_fnd: msleep(125); if (server->ssocket) { - sock_release(csocket); + sock_release(server->ssocket); server->ssocket = NULL; } /* buffer usually freed in free_mid - need to free it here on exit */ -- cgit v1.2.3 From 98bac62c9f1d6151dca7e8087aacce2e90fd43d3 Mon Sep 17 00:00:00 2001 From: Pavel Shilovsky Date: Mon, 1 Aug 2011 13:19:42 +0400 Subject: CIFS: Move RFC1002 check to a separate function Reviewed-and-Tested-by: Jeff Layton Signed-off-by: Pavel Shilovsky Signed-off-by: Steve French --- fs/cifs/connect.c | 116 +++++++++++++++++++++++++++++++----------------------- 1 file changed, 67 insertions(+), 49 deletions(-) (limited to 'fs') diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c index a2e9fcf4a99..2a53ade3e63 100644 --- a/fs/cifs/connect.c +++ b/fs/cifs/connect.c @@ -416,6 +416,63 @@ read_from_socket(struct TCP_Server_Info *server, struct msghdr *smb_msg, return rc; } +static bool +check_rfc1002_header(struct TCP_Server_Info *server, char *buf) +{ + char temp = *buf; + unsigned int pdu_length = be32_to_cpu( + ((struct smb_hdr *)buf)->smb_buf_length); + + /* + * The first byte big endian of the length field, + * is actually not part of the length but the type + * with the most common, zero, as regular data. + */ + if (temp == (char) RFC1002_SESSION_KEEP_ALIVE) { + return false; + } else if (temp == (char)RFC1002_POSITIVE_SESSION_RESPONSE) { + cFYI(1, "Good RFC 1002 session rsp"); + return false; + } else if (temp == (char)RFC1002_NEGATIVE_SESSION_RESPONSE) { + /* + * We get this from Windows 98 instead of an error on + * SMB negprot response. + */ + cFYI(1, "Negative RFC1002 Session Response Error 0x%x)", + pdu_length); + /* give server a second to clean up */ + msleep(1000); + /* + * Always try 445 first on reconnect since we get NACK + * on some if we ever connected to port 139 (the NACK + * is since we do not begin with RFC1001 session + * initialize frame). + */ + cifs_set_port((struct sockaddr *) + &server->dstaddr, CIFS_PORT); + cifs_reconnect(server); + wake_up(&server->response_q); + return false; + } else if (temp != (char) 0) { + cERROR(1, "Unknown RFC 1002 frame"); + cifs_dump_mem(" Received Data: ", buf, 4); + cifs_reconnect(server); + return false; + } + + /* else we have an SMB response */ + if ((pdu_length > CIFSMaxBufSize + MAX_CIFS_HDR_SIZE - 4) || + (pdu_length < sizeof(struct smb_hdr) - 1 - 4)) { + cERROR(1, "Invalid size SMB length %d pdu_length %d", + 4, pdu_length+4); + cifs_reconnect(server); + wake_up(&server->response_q); + return false; + } + + return true; +} + static int cifs_demultiplex_thread(void *p) { @@ -429,7 +486,6 @@ cifs_demultiplex_thread(void *p) struct list_head *tmp, *tmp2; struct task_struct *task_to_wake = NULL; struct mid_q_entry *mid_entry; - char temp; bool isLargeBuf = false; bool isMultiRsp; int rc; @@ -482,59 +538,21 @@ incomplete_rcv: else if (rc == 1) continue; - /* The right amount was read from socket - 4 bytes */ - /* so we can now interpret the length field */ - - /* the first byte big endian of the length field, - is actually not part of the length but the type - with the most common, zero, as regular data */ - temp = *buf; + /* + * The right amount was read from socket - 4 bytes, + * so we can now interpret the length field. + */ - /* Note that FC 1001 length is big endian on the wire, - but we convert it here so it is always manipulated - as host byte order */ + /* + * Note that RFC 1001 length is big endian on the wire, + * but we convert it here so it is always manipulated + * as host byte order. + */ pdu_length = be32_to_cpu(smb_buffer->smb_buf_length); cFYI(1, "rfc1002 length 0x%x", pdu_length+4); - - if (temp == (char) RFC1002_SESSION_KEEP_ALIVE) { - continue; - } else if (temp == (char)RFC1002_POSITIVE_SESSION_RESPONSE) { - cFYI(1, "Good RFC 1002 session rsp"); - continue; - } else if (temp == (char)RFC1002_NEGATIVE_SESSION_RESPONSE) { - /* we get this from Windows 98 instead of - an error on SMB negprot response */ - cFYI(1, "Negative RFC1002 Session Response Error 0x%x)", - pdu_length); - /* give server a second to clean up */ - msleep(1000); - /* always try 445 first on reconnect since we get NACK - * on some if we ever connected to port 139 (the NACK - * is since we do not begin with RFC1001 session - * initialize frame) - */ - cifs_set_port((struct sockaddr *) - &server->dstaddr, CIFS_PORT); - cifs_reconnect(server); - wake_up(&server->response_q); - continue; - } else if (temp != (char) 0) { - cERROR(1, "Unknown RFC 1002 frame"); - cifs_dump_mem(" Received Data: ", buf, length); - cifs_reconnect(server); - continue; - } - - /* else we have an SMB response */ - if ((pdu_length > CIFSMaxBufSize + MAX_CIFS_HDR_SIZE - 4) || - (pdu_length < sizeof(struct smb_hdr) - 1 - 4)) { - cERROR(1, "Invalid size SMB length %d pdu_length %d", - length, pdu_length+4); - cifs_reconnect(server); - wake_up(&server->response_q); + if (!check_rfc1002_header(server, buf)) continue; - } /* else length ok */ if (pdu_length > MAX_CIFS_SMALL_BUFFER_SIZE - 4) { -- cgit v1.2.3 From ad69bae178b86bf9f7e3f96d27492fba2052f187 Mon Sep 17 00:00:00 2001 From: Pavel Shilovsky Date: Mon, 1 Aug 2011 13:19:43 +0400 Subject: CIFS: Move mid search to a separate function Reviewed-and-Tested-by: Jeff Layton Signed-off-by: Pavel Shilovsky Signed-off-by: Steve French --- fs/cifs/connect.c | 126 ++++++++++++++++++++++++++---------------------------- 1 file changed, 61 insertions(+), 65 deletions(-) (limited to 'fs') diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c index 2a53ade3e63..12b2741ef8f 100644 --- a/fs/cifs/connect.c +++ b/fs/cifs/connect.c @@ -473,6 +473,64 @@ check_rfc1002_header(struct TCP_Server_Info *server, char *buf) return true; } +static struct mid_q_entry * +find_cifs_mid(struct TCP_Server_Info *server, struct smb_hdr *buf, + int *length, bool is_large_buf, bool *is_multi_rsp, char **bigbuf) +{ + struct mid_q_entry *mid = NULL, *tmp_mid, *ret = NULL; + + spin_lock(&GlobalMid_Lock); + list_for_each_entry_safe(mid, tmp_mid, &server->pending_mid_q, qhead) { + if (mid->mid != buf->Mid || + mid->midState != MID_REQUEST_SUBMITTED || + mid->command != buf->Command) + continue; + + if (*length == 0 && check2ndT2(buf, server->maxBuf) > 0) { + /* We have a multipart transact2 resp */ + *is_multi_rsp = true; + if (mid->resp_buf) { + /* merge response - fix up 1st*/ + *length = coalesce_t2(buf, mid->resp_buf); + if (*length > 0) { + *length = 0; + mid->multiRsp = true; + break; + } + /* All parts received or packet is malformed. */ + mid->multiEnd = true; + goto multi_t2_fnd; + } + if (!is_large_buf) { + /*FIXME: switch to already allocated largebuf?*/ + cERROR(1, "1st trans2 resp needs bigbuf"); + } else { + /* Have first buffer */ + mid->resp_buf = buf; + mid->largeBuf = true; + *bigbuf = NULL; + } + break; + } + mid->resp_buf = buf; + mid->largeBuf = is_large_buf; +multi_t2_fnd: + if (*length == 0) + mid->midState = MID_RESPONSE_RECEIVED; + else + mid->midState = MID_RESPONSE_MALFORMED; +#ifdef CONFIG_CIFS_STATS2 + mid->when_received = jiffies; +#endif + list_del_init(&mid->qhead); + ret = mid; + break; + } + spin_unlock(&GlobalMid_Lock); + + return ret; +} + static int cifs_demultiplex_thread(void *p) { @@ -487,7 +545,7 @@ cifs_demultiplex_thread(void *p) struct task_struct *task_to_wake = NULL; struct mid_q_entry *mid_entry; bool isLargeBuf = false; - bool isMultiRsp; + bool isMultiRsp = false; int rc; current->flags |= PF_MEMALLOC; @@ -589,72 +647,10 @@ incomplete_rcv: cifs_dump_mem("Bad SMB: ", buf, min_t(unsigned int, total_read, 48)); - mid_entry = NULL; server->lstrp = jiffies; - spin_lock(&GlobalMid_Lock); - list_for_each_safe(tmp, tmp2, &server->pending_mid_q) { - mid_entry = list_entry(tmp, struct mid_q_entry, qhead); - - if (mid_entry->mid != smb_buffer->Mid || - mid_entry->midState != MID_REQUEST_SUBMITTED || - mid_entry->command != smb_buffer->Command) { - mid_entry = NULL; - continue; - } - - if (length == 0 && - check2ndT2(smb_buffer, server->maxBuf) > 0) { - /* We have a multipart transact2 resp */ - isMultiRsp = true; - if (mid_entry->resp_buf) { - /* merge response - fix up 1st*/ - length = coalesce_t2(smb_buffer, - mid_entry->resp_buf); - if (length > 0) { - length = 0; - mid_entry->multiRsp = true; - break; - } else { - /* all parts received or - * packet is malformed - */ - mid_entry->multiEnd = true; - goto multi_t2_fnd; - } - } else { - if (!isLargeBuf) { - /* - * FIXME: switch to already - * allocated largebuf? - */ - cERROR(1, "1st trans2 resp " - "needs bigbuf"); - } else { - /* Have first buffer */ - mid_entry->resp_buf = - smb_buffer; - mid_entry->largeBuf = true; - bigbuf = NULL; - } - } - break; - } - mid_entry->resp_buf = smb_buffer; - mid_entry->largeBuf = isLargeBuf; -multi_t2_fnd: - 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 - list_del_init(&mid_entry->qhead); - break; - } - spin_unlock(&GlobalMid_Lock); - + mid_entry = find_cifs_mid(server, smb_buffer, &length, + isLargeBuf, &isMultiRsp, &bigbuf); if (mid_entry != NULL) { mid_entry->callback(mid_entry); /* Was previous buf put in mpx struct for multi-rsp? */ -- cgit v1.2.3 From 762dfd10573606c4afccd29267fcc79ec9f9599b Mon Sep 17 00:00:00 2001 From: Pavel Shilovsky Date: Mon, 1 Aug 2011 13:19:44 +0400 Subject: CIFS: Cleanup demupltiplex thread exiting code Reviewed-and-Tested-by: Jeff Layton Signed-off-by: Pavel Shilovsky Signed-off-by: Steve French --- fs/cifs/connect.c | 173 ++++++++++++++++++++++++++++++------------------------ 1 file changed, 96 insertions(+), 77 deletions(-) (limited to 'fs') diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c index 12b2741ef8f..80c2e3add3a 100644 --- a/fs/cifs/connect.c +++ b/fs/cifs/connect.c @@ -531,6 +531,101 @@ multi_t2_fnd: return ret; } +static void clean_demultiplex_info(struct TCP_Server_Info *server) +{ + int length; + + /* take it off the list, if it's not already */ + spin_lock(&cifs_tcp_ses_lock); + list_del_init(&server->tcp_ses_list); + spin_unlock(&cifs_tcp_ses_lock); + + spin_lock(&GlobalMid_Lock); + server->tcpStatus = CifsExiting; + spin_unlock(&GlobalMid_Lock); + wake_up_all(&server->response_q); + + /* + * Check if we have blocked requests that need to free. Note that + * cifs_max_pending is normally 50, but can be set at module install + * time to as little as two. + */ + spin_lock(&GlobalMid_Lock); + if (atomic_read(&server->inFlight) >= cifs_max_pending) + atomic_set(&server->inFlight, cifs_max_pending - 1); + /* + * We do not want to set the max_pending too low or we could end up + * with the counter going negative. + */ + spin_unlock(&GlobalMid_Lock); + /* + * Although there should not be any requests blocked on this queue it + * can not hurt to be paranoid and try to wake up requests that may + * haven been blocked when more than 50 at time were on the wire to the + * same server - they now will see the session is in exit state and get + * out of SendReceive. + */ + wake_up_all(&server->request_q); + /* give those requests time to exit */ + msleep(125); + + if (server->ssocket) { + sock_release(server->ssocket); + server->ssocket = NULL; + } + + if (!list_empty(&server->pending_mid_q)) { + struct list_head dispose_list; + struct mid_q_entry *mid_entry; + struct list_head *tmp, *tmp2; + + INIT_LIST_HEAD(&dispose_list); + spin_lock(&GlobalMid_Lock); + list_for_each_safe(tmp, tmp2, &server->pending_mid_q) { + mid_entry = list_entry(tmp, struct mid_q_entry, qhead); + cFYI(1, "Clearing mid 0x%x", mid_entry->mid); + mid_entry->midState = MID_SHUTDOWN; + list_move(&mid_entry->qhead, &dispose_list); + } + spin_unlock(&GlobalMid_Lock); + + /* now walk dispose list and issue callbacks */ + list_for_each_safe(tmp, tmp2, &dispose_list) { + mid_entry = list_entry(tmp, struct mid_q_entry, qhead); + cFYI(1, "Callback mid 0x%x", mid_entry->mid); + list_del_init(&mid_entry->qhead); + mid_entry->callback(mid_entry); + } + /* 1/8th of sec is more than enough time for them to exit */ + msleep(125); + } + + if (!list_empty(&server->pending_mid_q)) { + /* + * mpx threads have not exited yet give them at least the smb + * send timeout time for long ops. + * + * Due to delays on oplock break requests, we need to wait at + * least 45 seconds before giving up on a request getting a + * response and going ahead and killing cifsd. + */ + cFYI(1, "Wait for exit from demultiplex thread"); + msleep(46000); + /* + * If threads still have not exited they are probably never + * coming home not much else we can do but free the memory. + */ + } + + kfree(server->hostname); + kfree(server); + + length = atomic_dec_return(&tcpSesAllocCount); + if (length > 0) + mempool_resize(cifs_req_poolp, length + cifs_min_rcv, + GFP_KERNEL); +} + static int cifs_demultiplex_thread(void *p) { @@ -541,7 +636,6 @@ cifs_demultiplex_thread(void *p) struct smb_hdr *smb_buffer = NULL; struct msghdr smb_msg; struct kvec iov; - struct list_head *tmp, *tmp2; struct task_struct *task_to_wake = NULL; struct mid_q_entry *mid_entry; bool isLargeBuf = false; @@ -678,88 +772,13 @@ incomplete_rcv: } } /* end while !EXITING */ - /* take it off the list, if it's not already */ - spin_lock(&cifs_tcp_ses_lock); - list_del_init(&server->tcp_ses_list); - spin_unlock(&cifs_tcp_ses_lock); - - spin_lock(&GlobalMid_Lock); - server->tcpStatus = CifsExiting; - spin_unlock(&GlobalMid_Lock); - wake_up_all(&server->response_q); - - /* check if we have blocked requests that need to free */ - /* Note that cifs_max_pending is normally 50, but - can be set at module install time to as little as two */ - spin_lock(&GlobalMid_Lock); - if (atomic_read(&server->inFlight) >= cifs_max_pending) - atomic_set(&server->inFlight, cifs_max_pending - 1); - /* We do not want to set the max_pending too low or we - could end up with the counter going negative */ - spin_unlock(&GlobalMid_Lock); - /* Although there should not be any requests blocked on - this queue it can not hurt to be paranoid and try to wake up requests - that may haven been blocked when more than 50 at time were on the wire - to the same server - they now will see the session is in exit state - and get out of SendReceive. */ - wake_up_all(&server->request_q); - /* give those requests time to exit */ - msleep(125); - - if (server->ssocket) { - sock_release(server->ssocket); - server->ssocket = NULL; - } /* buffer usually freed in free_mid - need to free it here on exit */ cifs_buf_release(bigbuf); if (smallbuf) /* no sense logging a debug message if NULL */ cifs_small_buf_release(smallbuf); - if (!list_empty(&server->pending_mid_q)) { - struct list_head dispose_list; - - INIT_LIST_HEAD(&dispose_list); - spin_lock(&GlobalMid_Lock); - list_for_each_safe(tmp, tmp2, &server->pending_mid_q) { - mid_entry = list_entry(tmp, struct mid_q_entry, qhead); - cFYI(1, "Clearing mid 0x%x", mid_entry->mid); - mid_entry->midState = MID_SHUTDOWN; - list_move(&mid_entry->qhead, &dispose_list); - } - spin_unlock(&GlobalMid_Lock); - - /* now walk dispose list and issue callbacks */ - list_for_each_safe(tmp, tmp2, &dispose_list) { - mid_entry = list_entry(tmp, struct mid_q_entry, qhead); - cFYI(1, "Callback mid 0x%x", mid_entry->mid); - list_del_init(&mid_entry->qhead); - mid_entry->callback(mid_entry); - } - /* 1/8th of sec is more than enough time for them to exit */ - msleep(125); - } - - if (!list_empty(&server->pending_mid_q)) { - /* mpx threads have not exited yet give them - at least the smb send timeout time for long ops */ - /* due to delays on oplock break requests, we need - to wait at least 45 seconds before giving up - on a request getting a response and going ahead - and killing cifsd */ - cFYI(1, "Wait for exit from demultiplex thread"); - msleep(46000); - /* if threads still have not exited they are probably never - coming home not much else we can do but free the memory */ - } - - kfree(server->hostname); task_to_wake = xchg(&server->tsk, NULL); - kfree(server); - - length = atomic_dec_return(&tcpSesAllocCount); - if (length > 0) - mempool_resize(cifs_req_poolp, length + cifs_min_rcv, - GFP_KERNEL); + clean_demultiplex_info(server); /* if server->tsk was NULL then wait for a signal before exiting */ if (!task_to_wake) { -- cgit v1.2.3