Re: [PATCH] prevent possible infinite loop infs/select.c::do_pollfd()

From: Andrew Morton
Date: Sun Apr 24 2005 - 03:53:26 EST


Jesper Juhl <juhl-lkml@xxxxxx> wrote:
>
> If a sufficiently large 'num' is passed to the function, the for loop
> becomes an infinite loop - as far as I can see, that's a bug waiting to
> happen. Sure, 'len' in struct poll_list is currently an int, so currently
> this can't happen, but that might change in the future. In my oppinion,
> a function should be able to function correctly with the complete range
> of values that can potentially be passed via its parameters, and without
> the patch below that's just not true for this function.
>
> Signed-off-by: Jesper Juhl <juhl-lkml@xxxxxx>
>
> --- linux-2.6.12-rc2-mm3-orig/fs/select.c 2005-04-05 21:21:47.000000000 +0200
> +++ linux-2.6.12-rc2-mm3/fs/select.c 2005-04-24 01:11:13.000000000 +0200
> @@ -397,7 +397,7 @@ struct poll_list {
> static void do_pollfd(unsigned int num, struct pollfd * fdpage,
> poll_table ** pwait, int *count)
> {
> - int i;
> + unsigned int i;
>
> for (i = 0; i < num; i++) {
> int fd;

An expression such as the above which mixes signed and unsigned types will
promote the signed types to unsigned. So there is no bug in the above
`for' statement.

But there's a bug a bit further on:

> unsigned int mask;
> struct pollfd *fdp;
>
> mask = 0;
> fdp = fdpage+i;

This will oops the kernel if there are more than 2^31 pollfd's at *fdpage.

Yes, like most signed variables in the kernel, `i' should really be
unsigned, but I don't think it's worth raising a patch to change it.
-
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/