Re: [PATCH] Input: joydev - prevent potential read overflow in ioctl
From: Denis Efremov <hidden>
Date: 2021-07-03 11:21:37
Also in:
linux-input
Hi, CVE-2021-3612 was assigned to this patch. On 2/17/21 9:10 AM, Dan Carpenter wrote:
The problem here is that "len" might be less than "joydev->nabs" so the loops which verfy abspam[i] and keypam[] might read beyond the buffer.
The added check looks insufficient to me. There are second loops in these functions with unpatched "i < joydev->nabs;" and "i < joydev->nkey;" checks.
quoted hunk ↗ jump to hunk
Fixes: 999b874f4aa3 ("Input: joydev - validate axis/button maps before clobbering current ones") Signed-off-by: Dan Carpenter <redacted> --- drivers/input/joydev.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)diff --git a/drivers/input/joydev.c b/drivers/input/joydev.c index a2b5fbba2d3b..750f4513fe20 100644 --- a/drivers/input/joydev.c +++ b/drivers/input/joydev.c@@ -456,7 +456,7 @@ static int joydev_handle_JSIOCSAXMAP(struct joydev *joydev, if (IS_ERR(abspam)) return PTR_ERR(abspam); - for (i = 0; i < joydev->nabs; i++) { + for (i = 0; i < len && i < joydev->nabs; i++) { if (abspam[i] > ABS_MAX) { retval = -EINVAL; goto out;
memcpy(joydev->abspam, abspam, len);
for (i = 0; i < joydev->nabs; i++) // <== HERE
joydev->absmap[joydev->abspam[i]] = i;
quoted hunk ↗ jump to hunk
@@ -487,7 +487,7 @@ static int joydev_handle_JSIOCSBTNMAP(struct joydev *joydev, if (IS_ERR(keypam)) return PTR_ERR(keypam); - for (i = 0; i < joydev->nkey; i++) { + for (i = 0; i < (len / 2) && i < joydev->nkey; i++) { if (keypam[i] > KEY_MAX || keypam[i] < BTN_MISC) { retval = -EINVAL; goto out;
memcpy(joydev->keypam, keypam, len);
for (i = 0; i < joydev->nkey; i++) // <== HERE
joydev->keymap[keypam[i] - BTN_MISC] = i;
Also at the beginning of joydev_handle_JSIOCSAXMAP() there is a
len = min(len, sizeof(joydev->abspam));
abspam = memdup_user(argp, len);
Maybe we can call min(len, joydev->nabs) instead or even min3() and
use only len in the for loops then? Same for joydev_handle_JSIOCSBTNMAP.
Thanks,
Denis