On Sat, Dec 11, 2021 at 3:55 AM Dmitry Torokhov
<dmitry.torokhov@xxxxxxxxx> 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@xxxxxxxxxx>
> > ---
> > 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
