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

[Paul Jackson <pj@sgi.com>]

Andrew writes:
> Hate to be a bore, but I'm still waiting for a definitive patch ;)

OGAWA Hirofumi writes:
> Could you please post your lastest patch? It seems that my patch does
> not satisfy peoples.

Hmmm ...

As is often the case, Andrew knows more than he lets on.


OGAWA,

I cannot post a latest patch.  I do not know enough to do so.

I attempted some code readability comments, but probably I should not
have, since I don't have the knowledge of this code that I should have
in order to test it, nor to know by reading it what works.

You replied, a couple of messages ago, with entirely sensible comments
to my posting.  Since then, I have been doing my best to remove myself
from this thread.

Unless you have something else you wish to propose, please tell Andrew
to go with your patch, which (just to avoid confusion) I copy again
below.

Your patch satisfies me just fine now.

Please let Andrew know if he should take it.

My apologies for making a confusion of this.

Thank-you for your good work and your patience.


[PATCH] Fix NR_KEYS off-by-one error

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);

Signed-off-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
---

 drivers/char/vt_ioctl.c  |    6 ++++++
 include/linux/keyboard.h |    2 +-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff -puN include/linux/keyboard.h~nr_keys-off-by-one include/linux/keyboard.h
--- linux-2.6.8-rc2/include/linux/keyboard.h~nr_keys-off-by-one	2004-07-28 03:37:12.000000000 +0900
+++ linux-2.6.8-rc2-hirofumi/include/linux/keyboard.h	2004-07-28 03:37:12.000000000 +0900
@@ -16,7 +16,7 @@
 
 #define NR_SHIFT	9
 
-#define NR_KEYS		255
+#define NR_KEYS		256
 #define MAX_NR_KEYMAPS	256
 /* This means 128Kb if all keymaps are allocated. Only the superuser
 	may increase the number of keymaps beyond MAX_NR_OF_USER_KEYMAPS. */
diff -puN drivers/char/vt_ioctl.c~nr_keys-off-by-one drivers/char/vt_ioctl.c
--- linux-2.6.8-rc2/drivers/char/vt_ioctl.c~nr_keys-off-by-one	2004-07-29 01:31:12.000000000 +0900
+++ linux-2.6.8-rc2-hirofumi/drivers/char/vt_ioctl.c	2004-07-29 01:35:23.000000000 +0900
@@ -83,6 +83,12 @@ do_kdsk_ioctl(int cmd, struct kbentry __
 	if (copy_from_user(&tmp, user_kbe, sizeof(struct kbentry)))
 		return -EFAULT;
 
+#if NR_KEYS != 256 || MAX_NR_KEYMAPS != 256
+#error "you should check this too"
+	if (i >= NR_KEYS || s >= MAX_NR_KEYMAPS)
+		return -EINVAL;
+#endif
+
 	switch (cmd) {
 	case KDGKBENT:
 		key_map = key_maps[s];
_
-

-- 
                          I won't rest till it's the best ...
                          Programmer, Linux Scalability
                          Paul Jackson <pj@sgi.com> 1.650.933.1373