Re: [PATCH] cifs: Fix a kernel BUG with remote OS/2 server (try #3)

On Wed, 31 Mar 2010 12:00:03 +0530
Suresh Jayaraman <[email protected]> wrote:

> (Please consider for -stable once reviewed and accepted).
> 
> 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
> results in '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.
> 
> The workaround is to mask off high 16 bits if the number of bytes written as
> returned by the server is greater than the bytes requested by the client as
> suggested by Jeff Layton.
> 
> Cc: Jeff Layton <[email protected]>
> Signed-off-by: Suresh Jayaraman <[email protected]>
> ---
>  fs/cifs/cifssmb.c |   16 ++++++++++++++++
>  1 files changed, 16 insertions(+), 0 deletions(-)
> 
> diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
> index 7cc7f83..7d8ada8 100644
> --- a/fs/cifs/cifssmb.c
> +++ b/fs/cifs/cifssmb.c
> @@ -1517,6 +1517,14 @@ CIFSSMBWrite(const int xid, struct cifsTconInfo *tcon,
>  		*nbytes = le16_to_cpu(pSMBr->CountHigh);
>  		*nbytes = (*nbytes) << 16;
>  		*nbytes += le16_to_cpu(pSMBr->Count);
> +
> +		/*
> +		 * Mask off high 16 bits when bytes written as returned by the
> +		 * server is greater than bytes requested by the client. Some
> +		 * OS/2 servers are known to set incorrect CountHigh values.
> +		 */
> +		if (*nbytes > count)
> +			*nbytes &= 0xFFFF;
>  	}
>  
>  	cifs_buf_release(pSMB);
> @@ -1605,6 +1613,14 @@ CIFSSMBWrite2(const int xid, struct cifsTconInfo *tcon,
>  		*nbytes = le16_to_cpu(pSMBr->CountHigh);
>  		*nbytes = (*nbytes) << 16;
>  		*nbytes += le16_to_cpu(pSMBr->Count);
> +
> +		/*
> +		 * Mask off high 16 bits when bytes written as returned by the
> +		 * server is greater than bytes requested by the client. OS/2
> +		 * servers are known to set incorrect CountHigh values.
> +		 */
> +		if (*nbytes > count)
> +			*nbytes &= 0xFFFF;
>  	}
>  
>  /*	cifs_small_buf_release(pSMB); */ /* Freed earlier now in SendReceive2 */
> 

Looks good to me.

Reviewed-by: Jeff Layton <[email protected]>