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