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

[Andries Brouwer <aebr@win.tue.nl>]

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