Re: [PATCH] char: ppdev: fixed a validation issue

From: Vihas Mak
Date: Tue Nov 09 2021 - 10:35:16 EST


> What happens today if the mode is not set properly? Will the code paths
> error out eventually, or will the call succeed? The problem is that
> there might be code that is working today that would break with a change
> like this, as again, this is a very old driver.

I see. So I guess this driver might be better off without any changes,
as new changes might break things more severely.

Thanks,
Vihas

On Tue, Nov 9, 2021 at 4:53 PM Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
>
> On Tue, Nov 09, 2021 at 04:11:20PM +0530, Vihas Mak wrote:
> > > On Tue, Nov 09, 2021 at 12:28:18AM +0530, Vihas Mak wrote:
> > > > Make sure the mode is a valid IEEE1284 mode.
> > > What is a valid mode?
> >
> > The valid IEEE1284 modes are the ones defined in
> > <uapi/linux/parport.h>. Currently there are 10 modes. Namely nibble
> > mode, byte mode, ECP, ECPRLE, EPP and some specials.
>
> But what happens today if code has not been sending those values
> properly? This is a very old driver for very old hardware.
>
> > > How did you test this? And why is this needed now? What hardware was
> > > working that is now not going to work with this driver?
> >
> > I tested this on my local pc using a parallel port and read the
> > incoming data on my Raspberry PI.
> > I also used https://github.com/strezh/VPPSniffer. It's a simple
> > virtual parallel port used for debugging and sniffing.
> >
> > The mainline code wasn't validating the mode when a user-space program
> > does a ioctl call to change the current mode. It might
> > create some bugs if the new mode isn't one of the IEEE1284 modes
> > defined in <uapi/linux/parport.h>. So it's better to throw a EINVAL
> > beforehand, if the mode is invalid.
>
> What happens today if the mode is not set properly? Will the code paths
> error out eventually, or will the call succeed? The problem is that
> there might be code that is working today that would break with a change
> like this, as again, this is a very old driver.
>
> thanks,
>
> greg k-h