* [PATCH] CIFS: Fix too big maxBuf size for SMB3 mounts
@ 2014-02-14 9:31 Pavel Shilovsky
[not found] ` <1392370263-10364-1-git-send-email-piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
0 siblings, 1 reply; 5+ messages in thread
From: Pavel Shilovsky @ 2014-02-14 9:31 UTC (permalink / raw)
To: linux-cifs-u79uwXL29TY76Z2rM5mHXA
SMB3 servers can respond with MaxTransactSize of more than 4M
that can cause a memory allocation error returned from kmalloc
in a lock codepath. Also the client doesn't support multicredit
requests now and allows buffer sizes of 65536 bytes only. Set
MaxTransactSize to this maximum supported value.
Cc: stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org # 3.7+
Signed-off-by: Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
---
fs/cifs/smb2glob.h | 3 +++
fs/cifs/smb2ops.c | 14 ++++----------
fs/cifs/smb2pdu.c | 4 +++-
3 files changed, 10 insertions(+), 11 deletions(-)
diff --git a/fs/cifs/smb2glob.h b/fs/cifs/smb2glob.h
index c383508..bc0bb9c 100644
--- a/fs/cifs/smb2glob.h
+++ b/fs/cifs/smb2glob.h
@@ -57,4 +57,7 @@
#define SMB2_CMACAES_SIZE (16)
#define SMB3_SIGNKEY_SIZE (16)
+/* Maximum buffer size value we can send with 1 credit */
+#define SMB2_MAX_BUFFER_SIZE 65536
+
#endif /* _SMB2_GLOB_H */
diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index 757da3e..192f51a 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -182,11 +182,8 @@ smb2_negotiate_wsize(struct cifs_tcon *tcon, struct smb_vol *volume_info)
/* start with specified wsize, or default */
wsize = volume_info->wsize ? volume_info->wsize : CIFS_DEFAULT_IOSIZE;
wsize = min_t(unsigned int, wsize, server->max_write);
- /*
- * limit write size to 2 ** 16, because we don't support multicredit
- * requests now.
- */
- wsize = min_t(unsigned int, wsize, 2 << 15);
+ /* set it to the maximum buffer size value we can send with 1 credit */
+ wsize = min_t(unsigned int, wsize, SMB2_MAX_BUFFER_SIZE);
return wsize;
}
@@ -200,11 +197,8 @@ smb2_negotiate_rsize(struct cifs_tcon *tcon, struct smb_vol *volume_info)
/* start with specified rsize, or default */
rsize = volume_info->rsize ? volume_info->rsize : CIFS_DEFAULT_IOSIZE;
rsize = min_t(unsigned int, rsize, server->max_read);
- /*
- * limit write size to 2 ** 16, because we don't support multicredit
- * requests now.
- */
- rsize = min_t(unsigned int, rsize, 2 << 15);
+ /* set it to the maximum buffer size value we can send with 1 credit */
+ rsize = min_t(unsigned int, rsize, SMB2_MAX_BUFFER_SIZE);
return rsize;
}
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index 2013234..787e171 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -413,7 +413,9 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)
/* SMB2 only has an extended negflavor */
server->negflavor = CIFS_NEGFLAVOR_EXTENDED;
- server->maxBuf = le32_to_cpu(rsp->MaxTransactSize);
+ /* set it to the maximum buffer size value we can send with 1 credit */
+ server->maxBuf = min_t(unsigned int, le32_to_cpu(rsp->MaxTransactSize),
+ SMB2_MAX_BUFFER_SIZE);
server->max_read = le32_to_cpu(rsp->MaxReadSize);
server->max_write = le32_to_cpu(rsp->MaxWriteSize);
/* BB Do we need to validate the SecurityMode? */
--
1.7.10.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH] CIFS: Fix wrong pos argument of cifs_find_lock_conflict
[not found] ` <1392370263-10364-1-git-send-email-piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
@ 2014-02-14 9:31 ` Pavel Shilovsky
[not found] ` <1392370263-10364-2-git-send-email-piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
2014-02-14 19:10 ` [PATCH] CIFS: Fix too big maxBuf size for SMB3 mounts Jeff Layton
2014-02-14 22:51 ` Steve French
2 siblings, 1 reply; 5+ messages in thread
From: Pavel Shilovsky @ 2014-02-14 9:31 UTC (permalink / raw)
To: linux-cifs-u79uwXL29TY76Z2rM5mHXA
and use generic_file_aio_write rather than __generic_file_aio_write
in cifs_writev.
Signed-off-by: Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
---
fs/cifs/file.c | 24 ++++++------------------
1 file changed, 6 insertions(+), 18 deletions(-)
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index a7eda8e..d34dc3b 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -2539,31 +2539,19 @@ cifs_writev(struct kiocb *iocb, const struct iovec *iov,
struct cifsInodeInfo *cinode = CIFS_I(inode);
struct TCP_Server_Info *server = tlink_tcon(cfile->tlink)->ses->server;
ssize_t rc = -EACCES;
+ loff_t lock_pos = pos;
- BUG_ON(iocb->ki_pos != pos);
-
+ if (file->f_flags & O_APPEND)
+ lock_pos = i_size_read(inode);
/*
* We need to hold the sem to be sure nobody modifies lock list
* with a brlock that prevents writing.
*/
down_read(&cinode->lock_sem);
- if (!cifs_find_lock_conflict(cfile, pos, iov_length(iov, nr_segs),
+ if (!cifs_find_lock_conflict(cfile, lock_pos, iov_length(iov, nr_segs),
server->vals->exclusive_lock_type, NULL,
- CIFS_WRITE_OP)) {
- mutex_lock(&inode->i_mutex);
- rc = __generic_file_aio_write(iocb, iov, nr_segs,
- &iocb->ki_pos);
- mutex_unlock(&inode->i_mutex);
- }
-
- if (rc > 0) {
- ssize_t err;
-
- err = generic_write_sync(file, iocb->ki_pos - rc, rc);
- if (err < 0)
- rc = err;
- }
-
+ CIFS_WRITE_OP))
+ rc = generic_file_aio_write(iocb, iov, nr_segs, pos);
up_read(&cinode->lock_sem);
return rc;
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] CIFS: Fix too big maxBuf size for SMB3 mounts
[not found] ` <1392370263-10364-1-git-send-email-piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
2014-02-14 9:31 ` [PATCH] CIFS: Fix wrong pos argument of cifs_find_lock_conflict Pavel Shilovsky
@ 2014-02-14 19:10 ` Jeff Layton
2014-02-14 22:51 ` Steve French
2 siblings, 0 replies; 5+ messages in thread
From: Jeff Layton @ 2014-02-14 19:10 UTC (permalink / raw)
To: Pavel Shilovsky; +Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA
On Fri, 14 Feb 2014 13:31:02 +0400
Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org> wrote:
> SMB3 servers can respond with MaxTransactSize of more than 4M
> that can cause a memory allocation error returned from kmalloc
> in a lock codepath. Also the client doesn't support multicredit
> requests now and allows buffer sizes of 65536 bytes only. Set
> MaxTransactSize to this maximum supported value.
>
> Cc: stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org # 3.7+
> Signed-off-by: Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
> ---
> fs/cifs/smb2glob.h | 3 +++
> fs/cifs/smb2ops.c | 14 ++++----------
> fs/cifs/smb2pdu.c | 4 +++-
> 3 files changed, 10 insertions(+), 11 deletions(-)
>
> diff --git a/fs/cifs/smb2glob.h b/fs/cifs/smb2glob.h
> index c383508..bc0bb9c 100644
> --- a/fs/cifs/smb2glob.h
> +++ b/fs/cifs/smb2glob.h
> @@ -57,4 +57,7 @@
> #define SMB2_CMACAES_SIZE (16)
> #define SMB3_SIGNKEY_SIZE (16)
>
> +/* Maximum buffer size value we can send with 1 credit */
> +#define SMB2_MAX_BUFFER_SIZE 65536
> +
> #endif /* _SMB2_GLOB_H */
> diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
> index 757da3e..192f51a 100644
> --- a/fs/cifs/smb2ops.c
> +++ b/fs/cifs/smb2ops.c
> @@ -182,11 +182,8 @@ smb2_negotiate_wsize(struct cifs_tcon *tcon, struct smb_vol *volume_info)
> /* start with specified wsize, or default */
> wsize = volume_info->wsize ? volume_info->wsize : CIFS_DEFAULT_IOSIZE;
> wsize = min_t(unsigned int, wsize, server->max_write);
> - /*
> - * limit write size to 2 ** 16, because we don't support multicredit
> - * requests now.
> - */
> - wsize = min_t(unsigned int, wsize, 2 << 15);
> + /* set it to the maximum buffer size value we can send with 1 credit */
> + wsize = min_t(unsigned int, wsize, SMB2_MAX_BUFFER_SIZE);
>
> return wsize;
> }
> @@ -200,11 +197,8 @@ smb2_negotiate_rsize(struct cifs_tcon *tcon, struct smb_vol *volume_info)
> /* start with specified rsize, or default */
> rsize = volume_info->rsize ? volume_info->rsize : CIFS_DEFAULT_IOSIZE;
> rsize = min_t(unsigned int, rsize, server->max_read);
> - /*
> - * limit write size to 2 ** 16, because we don't support multicredit
> - * requests now.
> - */
> - rsize = min_t(unsigned int, rsize, 2 << 15);
> + /* set it to the maximum buffer size value we can send with 1 credit */
> + rsize = min_t(unsigned int, rsize, SMB2_MAX_BUFFER_SIZE);
>
> return rsize;
> }
> diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
> index 2013234..787e171 100644
> --- a/fs/cifs/smb2pdu.c
> +++ b/fs/cifs/smb2pdu.c
> @@ -413,7 +413,9 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)
>
> /* SMB2 only has an extended negflavor */
> server->negflavor = CIFS_NEGFLAVOR_EXTENDED;
> - server->maxBuf = le32_to_cpu(rsp->MaxTransactSize);
> + /* set it to the maximum buffer size value we can send with 1 credit */
> + server->maxBuf = min_t(unsigned int, le32_to_cpu(rsp->MaxTransactSize),
> + SMB2_MAX_BUFFER_SIZE);
> server->max_read = le32_to_cpu(rsp->MaxReadSize);
> server->max_write = le32_to_cpu(rsp->MaxWriteSize);
> /* BB Do we need to validate the SecurityMode? */
Acked-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] CIFS: Fix wrong pos argument of cifs_find_lock_conflict
[not found] ` <1392370263-10364-2-git-send-email-piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
@ 2014-02-14 19:28 ` Jeff Layton
0 siblings, 0 replies; 5+ messages in thread
From: Jeff Layton @ 2014-02-14 19:28 UTC (permalink / raw)
To: Pavel Shilovsky; +Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA, Al Viro
On Fri, 14 Feb 2014 13:31:03 +0400
Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org> wrote:
> and use generic_file_aio_write rather than __generic_file_aio_write
> in cifs_writev.
>
> Signed-off-by: Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
Al reported this, so you may want to add:
Reported-by: Al Viro <viro-3bDd1+5oDREiFSDQTTA3OLVCufUGDwFn@public.gmane.org>
...and cc him.
> ---
> fs/cifs/file.c | 24 ++++++------------------
> 1 file changed, 6 insertions(+), 18 deletions(-)
>
> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> index a7eda8e..d34dc3b 100644
> --- a/fs/cifs/file.c
> +++ b/fs/cifs/file.c
> @@ -2539,31 +2539,19 @@ cifs_writev(struct kiocb *iocb, const struct iovec *iov,
> struct cifsInodeInfo *cinode = CIFS_I(inode);
> struct TCP_Server_Info *server = tlink_tcon(cfile->tlink)->ses->server;
> ssize_t rc = -EACCES;
> + loff_t lock_pos = pos;
>
> - BUG_ON(iocb->ki_pos != pos);
> -
> + if (file->f_flags & O_APPEND)
> + lock_pos = i_size_read(inode);
Hmm...
I'll note that when we do the i_size_read in generic_write_checks, the
i_mutex is held. It's not held here though -- is that a potential race?
Could the i_size change after you check for locks but before you do the
I/O?
vfs.txt says:
"Adding and removing pages to/from an address_space is protected by the
inode's i_mutex."
That said, I don't have a great feel for how the locking rules work in
this regard...
> /*
> * We need to hold the sem to be sure nobody modifies lock list
> * with a brlock that prevents writing.
> */
> down_read(&cinode->lock_sem);
> - if (!cifs_find_lock_conflict(cfile, pos, iov_length(iov, nr_segs),
> + if (!cifs_find_lock_conflict(cfile, lock_pos, iov_length(iov, nr_segs),
> server->vals->exclusive_lock_type, NULL,
> - CIFS_WRITE_OP)) {
> - mutex_lock(&inode->i_mutex);
> - rc = __generic_file_aio_write(iocb, iov, nr_segs,
> - &iocb->ki_pos);
> - mutex_unlock(&inode->i_mutex);
> - }
> -
> - if (rc > 0) {
> - ssize_t err;
> -
> - err = generic_write_sync(file, iocb->ki_pos - rc, rc);
> - if (err < 0)
> - rc = err;
> - }
> -
> + CIFS_WRITE_OP))
> + rc = generic_file_aio_write(iocb, iov, nr_segs, pos);
> up_read(&cinode->lock_sem);
> return rc;
> }
--
Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] CIFS: Fix too big maxBuf size for SMB3 mounts
[not found] ` <1392370263-10364-1-git-send-email-piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
2014-02-14 9:31 ` [PATCH] CIFS: Fix wrong pos argument of cifs_find_lock_conflict Pavel Shilovsky
2014-02-14 19:10 ` [PATCH] CIFS: Fix too big maxBuf size for SMB3 mounts Jeff Layton
@ 2014-02-14 22:51 ` Steve French
2 siblings, 0 replies; 5+ messages in thread
From: Steve French @ 2014-02-14 22:51 UTC (permalink / raw)
To: Pavel Shilovsky; +Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA
Merged into cifs-2.6.git for-next
On Fri, Feb 14, 2014 at 3:31 AM, Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org> wrote:
> SMB3 servers can respond with MaxTransactSize of more than 4M
> that can cause a memory allocation error returned from kmalloc
> in a lock codepath. Also the client doesn't support multicredit
> requests now and allows buffer sizes of 65536 bytes only. Set
> MaxTransactSize to this maximum supported value.
>
> Cc: stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org # 3.7+
> Signed-off-by: Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
> ---
> fs/cifs/smb2glob.h | 3 +++
> fs/cifs/smb2ops.c | 14 ++++----------
> fs/cifs/smb2pdu.c | 4 +++-
> 3 files changed, 10 insertions(+), 11 deletions(-)
>
> diff --git a/fs/cifs/smb2glob.h b/fs/cifs/smb2glob.h
> index c383508..bc0bb9c 100644
> --- a/fs/cifs/smb2glob.h
> +++ b/fs/cifs/smb2glob.h
> @@ -57,4 +57,7 @@
> #define SMB2_CMACAES_SIZE (16)
> #define SMB3_SIGNKEY_SIZE (16)
>
> +/* Maximum buffer size value we can send with 1 credit */
> +#define SMB2_MAX_BUFFER_SIZE 65536
> +
> #endif /* _SMB2_GLOB_H */
> diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
> index 757da3e..192f51a 100644
> --- a/fs/cifs/smb2ops.c
> +++ b/fs/cifs/smb2ops.c
> @@ -182,11 +182,8 @@ smb2_negotiate_wsize(struct cifs_tcon *tcon, struct smb_vol *volume_info)
> /* start with specified wsize, or default */
> wsize = volume_info->wsize ? volume_info->wsize : CIFS_DEFAULT_IOSIZE;
> wsize = min_t(unsigned int, wsize, server->max_write);
> - /*
> - * limit write size to 2 ** 16, because we don't support multicredit
> - * requests now.
> - */
> - wsize = min_t(unsigned int, wsize, 2 << 15);
> + /* set it to the maximum buffer size value we can send with 1 credit */
> + wsize = min_t(unsigned int, wsize, SMB2_MAX_BUFFER_SIZE);
>
> return wsize;
> }
> @@ -200,11 +197,8 @@ smb2_negotiate_rsize(struct cifs_tcon *tcon, struct smb_vol *volume_info)
> /* start with specified rsize, or default */
> rsize = volume_info->rsize ? volume_info->rsize : CIFS_DEFAULT_IOSIZE;
> rsize = min_t(unsigned int, rsize, server->max_read);
> - /*
> - * limit write size to 2 ** 16, because we don't support multicredit
> - * requests now.
> - */
> - rsize = min_t(unsigned int, rsize, 2 << 15);
> + /* set it to the maximum buffer size value we can send with 1 credit */
> + rsize = min_t(unsigned int, rsize, SMB2_MAX_BUFFER_SIZE);
>
> return rsize;
> }
> diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
> index 2013234..787e171 100644
> --- a/fs/cifs/smb2pdu.c
> +++ b/fs/cifs/smb2pdu.c
> @@ -413,7 +413,9 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)
>
> /* SMB2 only has an extended negflavor */
> server->negflavor = CIFS_NEGFLAVOR_EXTENDED;
> - server->maxBuf = le32_to_cpu(rsp->MaxTransactSize);
> + /* set it to the maximum buffer size value we can send with 1 credit */
> + server->maxBuf = min_t(unsigned int, le32_to_cpu(rsp->MaxTransactSize),
> + SMB2_MAX_BUFFER_SIZE);
> server->max_read = le32_to_cpu(rsp->MaxReadSize);
> server->max_write = le32_to_cpu(rsp->MaxWriteSize);
> /* BB Do we need to validate the SecurityMode? */
> --
> 1.7.10.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Thanks,
Steve
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-02-14 22:51 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-14 9:31 [PATCH] CIFS: Fix too big maxBuf size for SMB3 mounts Pavel Shilovsky
[not found] ` <1392370263-10364-1-git-send-email-piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
2014-02-14 9:31 ` [PATCH] CIFS: Fix wrong pos argument of cifs_find_lock_conflict Pavel Shilovsky
[not found] ` <1392370263-10364-2-git-send-email-piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
2014-02-14 19:28 ` Jeff Layton
2014-02-14 19:10 ` [PATCH] CIFS: Fix too big maxBuf size for SMB3 mounts Jeff Layton
2014-02-14 22:51 ` Steve French
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.