Re: [Coverity] Untrusted user data in kernel

[Jan Harkes <jaharkes@cs.cmu.edu>]

On Wed, Jan 05, 2005 at 10:04:23AM -0200, Marcelo Tosatti wrote:
> On Thu, Dec 16, 2004 at 05:33:32PM -0800, Bryan Fulton wrote:
> Correct, fix for both v2.4 and v2.6 attached. Adds bound checking:
> 
> Jan Harkes, please check correctness so we can apply it.

I was looking at this and actually think that both in_size and out_size
should just be changed to unsigned short instead of signed short (the
values should never be negative, period).

That fixes the bounds checking on the in_size, but the out_size one is
still questionable. I'm not even sure the code is actually testing the
right thing there.

> --- linux-2.6.10-rc3/fs/coda/upcall.c.orig	2005-01-05 10:30:24.575445152 -0200
> +++ linux-2.6.10-rc3/fs/coda/upcall.c	2005-01-05 10:30:26.623133856 -0200
> @@ -550,10 +550,15 @@
>  	UPARG(CODA_IOCTL);
>  
>          /* build packet for Venus */
> -        if (data->vi.in_size > VC_MAXDATASIZE) {
> +        if (data->vi.in_size > VC_MAXDATASIZE || data->vi.in_size < 0) {
>  		error = -EINVAL;
>  		goto exit;
> -        }
> +        } 

This part would work, but changing to the variable to unsigned short
in include/linux/coda.h works just as well.

> +	if (data->vi.out_size > VC_MAXDATASIZE || data->vi.out_size < 0) {
> +		error = -EINVAL;
> +		goto exit;
> +	}

We might be overwriting out_size when making the upcall to venus, so
checking out_size here probably doesn't help all that much. I'm still
looking at what exactly is going on with that. I should have a patch by
the end of the week.

Jan