Re: [PATCH] Input: joydev - prevent potential write out of bounds in ioctl

On Sun, Jun 20, 2021 at 5:01 AM Alexander Larkin [off-list ref] wrote:

    The problem is that the check of user input values that is just
    before the fixed line of code is for the part of first values
    (before len or before len/2), but then the usage of all the values
    including i >= len (or i >= len/2) could be.

No, I think the problem is simpler than that.

This part is unnecessary - all values of "joydev->abspam[i]" have been
validated (either they are the old ones, or the new ones that we just
validated).

The problem here is not that we walk past "len/2", but that the code
*should* have used

        joydev->keymap[joydev->keypam[i] - BTN_MISC] = i;

(note the "keypam[1]" vs "joydev->keypam[i]").

And the reason it *should* walk the whole "joydev->nkey" is that if
there are later cases with the same keypam value, the later ones
should override the previous ones (well, that "should" is more a
"traditionally have").

So I think the right patch is this one-liner

  diff --git a/drivers/input/joydev.c b/drivers/input/joydev.c
  index da8963a9f044..947d440a3be6 100644
  --- a/drivers/input/joydev.c
  +++ b/drivers/input/joydev.c
  @@ -499,7 +499,7 @@ static int joydev_handle_JSIOCSBTNMAP(struct
joydev *joydev,
        memcpy(joydev->keypam, keypam, len);

        for (i = 0; i < joydev->nkey; i++)
  -             joydev->keymap[keypam[i] - BTN_MISC] = i;
  +             joydev->keymap[joydev->keypam[i] - BTN_MISC] = i;

    out:
        kfree(keypam);

(whitespace-damaged, I would like Dmitry to think about it rather than
apply this mindlessly.

Dmitry?

              Linus