mbox series

[0/2] media: dvb-usbv2: Prevent usb race condition, buffer overflow az6007

Message ID 20250421-ubsan-out-of-sub-v1-0-d9239a5af007@arnaud-lcm.com
Headers show
Series media: dvb-usbv2: Prevent usb race condition, buffer overflow az6007 | expand

Message

Arnaud Lecomte April 21, 2025, 4:33 p.m. UTC 
Hi,

Background
----------

The current implementation of the function az6007_i2c_xfer does not
check whether the I2C messages it receive are not larger than the
buffer used to transfer them to the related usb device.

Moreover, in a multi-threaded scenario where a thread disconnect the USB
device while an other thread is in az6007_i2c_xfer, we can end-up with
an use after-free of the device. The reproducer from syzbot is executing
his tests in this multi-threaded context:
[  156.847620][  T779] usb write operation failed. (-71)
[  156.852064][  T779] usb 2-1: dvb_usb_v2: will pass the complete MPEG2 transport stream to the software demuxer
[  156.853219][  T779] dvbdev: DVB: registering new adapter (Terratec H7)
[  156.853880][  T779] usb 2-1: media controller created
[  156.855421][  T779] usb read operation failed. (-71)
[  156.856123][  T779] usb write operation failed. (-71)
[  156.862775][  T779] dvb_usb_az6007 2-1:0.0: probe with driver dvb_usb_az6007 failed with error -5
[  156.865923][  T779] usb 2-1: USB disconnect, device number 28
[  157.008077][   T52] usb write operation failed. (-71)
[  157.011674][   T52] usb 4-1: dvb_usb_v2: will pass the complete MPEG2 transport stream to the software demuxer
[  157.012774][   T52] dvbdev: DVB: registering new adapter (Terratec H7)
[  157.013404][   T52] usb 4-1: media controller created
[  157.014212][   T52] usb read operation failed. (-71)
[  157.014915][   T52] usb write operation failed. (-71)
[  157.020729][   T52] dvb_usb_az6007 4-1:0.0: probe with driver dvb_usb_az6007 failed with error -5
[  157.025082][   T52] usb 4-1: USB disconnect, device number 28

The following serie of patch has been divided in 2 patches:
 - First patch: Add bound checking for i2c message length before filling
   the device buffer.
 - Second patch: Lock the usb device when starting the transfer to avoid
   the device being disconnected during it.

Considerations
----------
Concerning the upper bound checking for i2c messages, we could maybe be more
strict. I've currently set the limit to 4090 bytes to avoid the buffer
overflow but as mentionned, I2C messages are usually way more little. Be
more restrictive on this limit could be beneficial.

Concerning the race condition issue, maybe there is a more efficient
solution than locking the usb device.

Arnaud

Reported-by: syzbot+0192952caa411a3be209@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=0192952caa411a3be209
Fixes: ac71fabf1567 ("gcc-15: work around sequence-point warning")
Signed-off-by: Arnaud Lecomte <contact@arnaud-lcm.com>
---
Arnaud Lecomte (2):
      media: dvb-usbv2: add bound checking for messages length in i2c_xfer
      media: dvb-usbv2: ensure safe USB transfers on disconnect in i2c_xfer

 drivers/media/usb/dvb-usb-v2/az6007.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)
---
base-commit: 9d7a0577c9db35c4cc52db90bc415ea248446472
change-id: 20250421-ubsan-out-of-sub-96b26a47d49c

Best regards,