[linux-cifs-client] [PATCH] cifs: Fix a kernel BUG with remote OS/2 server
Jeff Layton
jlayton at samba.org
Tue Mar 30 12:35:43 MDT 2010
On Tue, 30 Mar 2010 23:46:07 +0530
Suresh Jayaraman <sjayaraman at suse.de> wrote:
> Sorry about one more iteration -- fixed a few checkpatch warnings.
>
> While chasing a bug report involving a OS/2 server, I noticed the server sets
> pSMBr->CountHigh to a incorrect value even in case of normal writes. This
> leads to 'nbytes' being computed wrongly and triggers a kernel BUG at
> mm/filemap.c.
>
> void iov_iter_advance(struct iov_iter *i, size_t bytes)
> {
> BUG_ON(i->count < bytes); <--- BUG here
>
> Why the server is setting 'CountHigh' is not clear but only does so after
> writing 64k bytes. Though this looks like the server bug, the client side
> crash may not be acceptable.
>
> When the write returns successfully and the bytes written as returned by
> the server is greater than bytes requested by the client, reset it to original
> bytes requested.
>
>
> Cc: Jeff Layton <jlayton at samba.org>
> Signed-off-by: Suresh Jayaraman <sjayaraman at suse.de>
> ---
> fs/cifs/cifssmb.c | 20 ++++++++++++++++++++
> 1 files changed, 20 insertions(+), 0 deletions(-)
>
> diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
> index 7cc7f83..5872e58 100644
> --- a/fs/cifs/cifssmb.c
> +++ b/fs/cifs/cifssmb.c
> @@ -1517,6 +1517,16 @@ CIFSSMBWrite(const int xid, struct cifsTconInfo *tcon,
> *nbytes = le16_to_cpu(pSMBr->CountHigh);
> *nbytes = (*nbytes) << 16;
> *nbytes += le16_to_cpu(pSMBr->Count);
> +
> + /*
> + * Workaround: Some legacy servers (read OS/2) might incorrectly
> + * set CountHigh for normal writes resulting in wrong 'nbytes'
> + * value. So when the write returns successfully and the bytes
> + * written as returned by the server is greater than bytes
> + * requested, reset it to original bytes requested.
> + */
> + if (*nbytes > count)
> + *nbytes = count;
> }
>
> cifs_buf_release(pSMB);
> @@ -1605,6 +1615,16 @@ CIFSSMBWrite2(const int xid, struct cifsTconInfo *tcon,
> *nbytes = le16_to_cpu(pSMBr->CountHigh);
> *nbytes = (*nbytes) << 16;
> *nbytes += le16_to_cpu(pSMBr->Count);
> +
> + /* Workaround: Some legacy servers (read OS/2) might incorrectly
> + * set CountHigh for normal writes resulting in wrong 'nbytes'
> + * value. So when the write returns successfully and the bytes
> + * written as returned by the server is greater than bytes
> + * requested, reset it to original bytes requested.
> + */
> +
> + if (*nbytes > count)
> + *nbytes = count;
> }
>
> /* cifs_small_buf_release(pSMB); */ /* Freed earlier now in SendReceive2 */
>
So what happens if the server returned less than "count" bytes in the
->Count field and put junk in the CountHigh? You'll now be returning
that "count" bytes were written when they really weren't.
I don't think that's what you want. I think Steve was suggesting
that we don't use "CountHigh" when that would make it look like the
server wrote more than was requested. I think rather than setting
*nbytes to count, you should just mask off the upper 16 bits in *nbytes.
--
Jeff Layton <jlayton at samba.org>
More information about the linux-cifs-client
mailing list