On Sat, Jul 03, 2021 at 02:20:13PM +0300, Denis Efremov wrote: > 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. > Thanks, Denis. You're right. The fix isn't complete. We discussed this in a different thread but I sort of assumed it was dealt with and didn't follow up. :/ https://lore.kernel.org/lkml/20210620120030.1513655-1-avlarkin82@xxxxxxxxx/ regards, dan carpenter > > > > Fixes: 999b874f4aa3 ("Input: joydev - validate axis/button maps before clobbering current ones") > > Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx> > > --- > > 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; > > > > @@ -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
