# Re: \[PATCH\] Input: psmouse: check the result of PSMOUSE\_CMD\_GET\* commands

- From: Alexander Potapenko \<glider@google.com\>
- Date: 2021-12-13 11:27:23 
- List: linux-input
- Page: https://ratatoskr.run/linux-input/2021/12/8933687
- Thread: https://ratatoskr.run/linux-input/2021/11/8933602/t.md

```text
On Sat, Dec 11, 2021 at 3:55 AM Dmitry Torokhov
[ref] wrote:
>
> Hi Alexander,
>
> On Mon, Nov 29, 2021 at 03:38:45PM +0100, Alexander Potapenko wrote:
> > Execution of a PSMOUSE_CMD_GET* command may fail, leaving the output
> > buffer uninitialized. Make sure to check the return value of
> > ps2_command() and bail out before checking the buffer contents.
> >
> > The use of uninitialized data in genius_detect() was detected by KMSAN,
> > other places were fixed for the sake of uniformity.
> >
> > Signed-off-by: Alexander Potapenko <glider@google.com>
> > ---
> >  drivers/input/mouse/psmouse-base.c | 21 ++++++++++++++++-----
> >  1 file changed, 16 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c
> > index 0b4a3039f312f..a3305653ce891 100644
> > --- a/drivers/input/mouse/psmouse-base.c
> > +++ b/drivers/input/mouse/psmouse-base.c
> > @@ -546,13 +546,16 @@ static int genius_detect(struct psmouse *psmouse, bool set_properties)
> >  {
> >       struct ps2dev *ps2dev = &psmouse->ps2dev;
> >       u8 param[4];
> > +     int error;
> >
> >       param[0] = 3;
> >       ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES);
> >       ps2_command(ps2dev,  NULL, PSMOUSE_CMD_SETSCALE11);
> >       ps2_command(ps2dev,  NULL, PSMOUSE_CMD_SETSCALE11);
> >       ps2_command(ps2dev,  NULL, PSMOUSE_CMD_SETSCALE11);
> > -     ps2_command(ps2dev, param, PSMOUSE_CMD_GETINFO);
> > +     error = ps2_command(ps2dev, param, PSMOUSE_CMD_GETINFO);
> > +     if (error)
> > +             return error;
>
> If we care about this I think we should care about errors from the rest
> of ps2_command()s. Otherwise we might have buffer initialized, but we
> are still checking potential garbage in it.

Ok, I am going to add error checking to ps2_command()s that occur in
the device detection code, because it's a sane assumption that every
command in the device handshake should succeed.
The only exception I see is the IntelliMouse 4.0 support code in
im_explorer_detect
(https://elixir.bootlin.com/linux/latest/source/drivers/input/mouse/psmouse-base.c#L628),
where it is unclear whether these functions must succeed or not.
I also won't be touching calls to ps2_command() in void functions
which do not return errors anyway, and PSMOUSE_CMD_RESET_DIS commands,
for which it is also unclear what to return.


I think it makes sense to bail out if one of the ps2_command()
preceding PSMOUSE_CMD_GET* returned an error, but am not sure

The bugs caused by incorrect uses of PSMOUSE_CMD_GET are blocking
KMSAN builds on syzbot, so we can immediately test the effect of the
proposed change.

If there were


> Thanks.
>
> --
> Dmitry



-- 
Alexander Potapenko
Software Engineer

Google Germany GmbH
Erika-Mann-Straße, 33
80636 München

Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
```