Re: [PATCH 3/5] cifs: sanitize length checking in coalesce_t2

[David Howells <dhowells-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]

Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:

> -	__u16 byte_count, total_data_size, total_in_buf, total_in_buf2;
> +	unsigned int byte_count, total_in_buf;
> +	__u16 total_data_size, total_in_buf2;

There's no particular need for any of these to be __u16; I'd recommend making
them all unsigned int or size_t.

> +	/* did this field "wrap" ? */
> +	if (total_in_buf & ~((1<<16)-1))
> +		return -EINVAL;

I'd recommend something more like the following:

	/* check the server isn't offering too much data */
	if (total_in_buf > USHRT_MAX)
		return -EINVAL;

rather than calculating a mask.

Also, would EPROTO be a better choice for packet parsing errors than EINVAL?

> +	/* did this field "wrap" ? */
> +	if (byte_count & ~((1<<16)-1))
> +		return -EINVAL;

Ditto.

David