linux-kernel - Re: [PATCH v2] usb: using mutex lock and supporting O_NONBLOCK flag in iowarrior

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <2024101654-kebab-pastrami-a6b8@gregkh>
Date: Wed, 16 Oct 2024 17:34:51 +0200
From: Greg KH <gregkh@...uxfoundation.org>
To: Jeongjun Park <aha310510@...il.com>
Cc: oneukum@...e.com, colin.i.king@...il.com, linux-usb@...r.kernel.org,
	linux-kernel@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH v2] usb: using mutex lock and supporting O_NONBLOCK flag
 in iowarrior_read()

On Wed, Oct 16, 2024 at 11:52:47PM +0900, Jeongjun Park wrote:
> Jeongjun Park <aha310510@...il.com> 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@...r.kernel.org
> > Fixes: 946b960d13c1 ("USB: add driver for iowarrior devices.")
> > Signed-off-by: Jeongjun Park <aha310510@...il.com>
> 
> 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