Re: [PATCH v2] usb: using mutex lock and supporting O_NONBLOCK flag in iowarrior_read()

[Greg KH]

On Wed, Oct 16, 2024 at 11:52:47PM +0900, Jeongjun Park wrote:
> Jeongjun Park <aha310510@xxxxxxxxx> wrote:
> >
> > iowarrior_read() uses the iowarrior dev structure, but does not use any
> > lock on the structure. This can cause various bugs including data-races,
> > so it is more appropriate to use a mutex lock to safely protect the
> > iowarrior dev structure. When using a mutex lock, you should split the
> > branch to prevent blocking when the O_NONBLOCK flag is set.
> >
> > In addition, it is unnecessary to check for NULL on the iowarrior dev
> > structure obtained by reading file->private_data. Therefore, it is
> > better to remove the check.
> >
> > Cc: stable@xxxxxxxxxxxxxxx
> > Fixes: 946b960d13c1 ("USB: add driver for iowarrior devices.")
> > Signed-off-by: Jeongjun Park <aha310510@xxxxxxxxx>
> 
> I think this patch should be moved to the usb-linus tree to be applied in the
> next rc version. iowarrior_read() is very vulnerable to a data-race because it
> reads a struct iowarrior without a mutex_lock. I think this almost certainly
> leads to a data-race, so I think this function should be moved to the
> usb-linus tree to be fixed as soon as possible.
> 
> I would appreciate it if you could review this.

Do you have this hardware to test this with?  What type of data race
will happen for a normal user of it?  What systems that have this
hardware allow untrusted users to operate this hardware?

thanks,

greg k-h