Re: [PATCH] Fix NR_KEYS off-by-one error

From: Andries Brouwer
Date: Wed Jul 28 2004 - 06:53:52 EST


On Sat, Jul 17, 2004 at 03:23:27PM +0900, OGAWA Hirofumi wrote:

> > :: KDGKBENT ioctl can use 256 entries (0-255), but it was defined as
> > :: key_map[NR_KEYS] (NR_KEYS == 255). The code seems also thinking it's 256.
> > ::
> > :: key_map[0] = U(K_ALLOCATED);
> > :: for (j = 1; j < NR_KEYS; j++)
> > :: key_map[j] = U(K_HOLE);
> >
> > I think the code has no opinion. It was 128 in 2.4.
> > I am not aware of assumptions on NR_KEYS.
> > So, do not think this is an off-by-one error.
>
> My point is that key_map is 0-254 array. But KDGKBENT uses 255
>
> case KDGKBENT:
> key_map = key_maps[s];
> if (key_map) {
> val = U(key_map[i]);

Hmm. Yes.

In 2.4 the preceding code is

if (i >= NR_KEYS || s >= MAX_NR_KEYMAPS)
return -EINVAL;

It looks like somebody removed this test in 2.6. Bad.
No doubt an error caused by "fixing" gcc warnings.
Shame on akpm.

When an array has an arbitrary upper bound that can be changed
via a #define, and for some values of the upper bound a test
is superfluous, that does not mean that the test is superfluous.

Andries

-
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/