Re: [PATCH] cifs: fix bad buffer length check in coalesce_t2

[Shirish Pargaonkar]

On Sun, Jan 1, 2012 at 9:34 AM, Jeff Layton <jlayton@xxxxxxxxxx> wrote:
> The current check looks to see if the RFC1002 length is larger than
> CIFSMaxBufSize, and fails if it is. The buffer is actually larger than
> that by MAX_CIFS_HDR_SIZE.
>
> This bug has been around for a long time, but the fact that we used to
> cap the clients MaxBufferSize at the same level as the server tended
> to paper over it. Commit c974befa changed that however and caused this
> bug to bite in more cases.
>
> Reported-and-Tested-by: Konstantinos Skarlatos <k.skarlatos@xxxxxxxxx>
> Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
> ---
>  fs/cifs/connect.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 8cd4b52..27c4f25 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -282,7 +282,7 @@ static int coalesce_t2(struct smb_hdr *psecond, struct smb_hdr *pTargetSMB)
>        byte_count = be32_to_cpu(pTargetSMB->smb_buf_length);
>        byte_count += total_in_buf2;
>        /* don't allow buffer to overflow */
> -       if (byte_count > CIFSMaxBufSize)
> +       if (byte_count > CIFSMaxBufSize + MAX_CIFS_HDR_SIZE - 4)
>                return -ENOBUFS;
>        pTargetSMB->smb_buf_length = cpu_to_be32(byte_count);
>
> --
> 1.7.7.4
>

This looks correct.  But the way Windows XP server responds to request
 trans2/find_first2/info level is different than how Windows 2003 Server
and Windows 2008 Server respond.

So a related concern would be, for a response from Windows XP server,
this check in function check2ndT2 does not make sense.
 if (total_data_size > CIFSMaxBufSize) {

It is possible to have large number of entries in a directory such that the
response to a  ls  command can exceed CIFSMaxBufSize.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/