Re: [PATCH] Input: joydev - prevent potential read overflow in ioctl

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

On Fri, Feb 19, 2021 at 11:32:15AM +0300, Dan Carpenter wrote:
> On Thu, Feb 18, 2021 at 07:17:44PM -0800, Dmitry Torokhov wrote:
> > Hi Dan,
> > 
> > On Wed, Feb 17, 2021 at 09:10:15AM +0300, 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.
> > > 
> > > Fixes: 999b874f4aa3 ("Input: joydev - validate axis/button maps before clobbering current ones")
> > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > 
> > Thank you for the patch.
> > 
> > > ---
> > >  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;
> > > @@ -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++) {
> > 
> > 
> > I think we also need to make sure that len is even. Do you mind if I add
> > the check at the top of the function to return -EINVAL if it is odd?
> 
> Yes.  You are right.
> 
> Btw, thank you for fixing my patches, but feel free to just ask me to
> resend if that's easier for you.

It is not really fixing your patch, rather addressing another issue
while we are there. Some might even say it could have been a separate
patch.

Anyway, for small fixups like this I feel it's less noise on the lists
without extra resubmissions, that is why I often do them on my side.

Thanks!

-- 
Dmitry