lkml.org 
[lkml]   [2005]   [Jan]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [Coverity] Untrusted user data in kernel
From
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

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-03-22 14:09    [W:0.783 / U:0.240 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site