| Message ID | 20210819104221.27122-1-paskripkin@gmail.com |
|---|---|
| State | Accepted |
| Commit | 44870a9e7a3c24acbb3f888b2a7cc22c9bdf7e7f |
| Headers | show |
| Series | [v3] media: mxl111sf: change mutex_init() location | expand |
On 8/19/21 13:42, Pavel Skripkin wrote: > Syzbot reported, that mxl111sf_ctrl_msg() uses uninitialized > mutex. The problem was in wrong mutex_init() location. > > Previous mutex_init(&state->msg_lock) call was in ->init() function, but > dvb_usbv2_init() has this order of calls: > > dvb_usbv2_init() > dvb_usbv2_adapter_init() > dvb_usbv2_adapter_frontend_init() > props->frontend_attach() > > props->init() > > Since mxl111sf_* devices call mxl111sf_ctrl_msg() in ->frontend_attach() > internally we need to initialize state->msg_lock before > frontend_attach(). To achieve it, ->probe() call added to all mxl111sf_* > devices, which will simply initiaize mutex. > > Reported-and-tested-by: syzbot+5ca0bf339f13c4243001@syzkaller.appspotmail.com > Fixes: 8572211842af ("[media] mxl111sf: convert to new DVB USB") > Signed-off-by: Pavel Skripkin <paskripkin@gmail.com> Hi, Sean! Did you have a chance to review this patch? Thank you :) With regards, Pavel Skripkin
On Sun, Sep 12, 2021 at 06:49:52PM +0300, Pavel Skripkin wrote: > On 8/19/21 13:42, Pavel Skripkin wrote: > > Syzbot reported, that mxl111sf_ctrl_msg() uses uninitialized > > mutex. The problem was in wrong mutex_init() location. > > > > Previous mutex_init(&state->msg_lock) call was in ->init() function, but > > dvb_usbv2_init() has this order of calls: > > > > dvb_usbv2_init() > > dvb_usbv2_adapter_init() > > dvb_usbv2_adapter_frontend_init() > > props->frontend_attach() > > > > props->init() > > > > Since mxl111sf_* devices call mxl111sf_ctrl_msg() in ->frontend_attach() > > internally we need to initialize state->msg_lock before > > frontend_attach(). To achieve it, ->probe() call added to all mxl111sf_* > > devices, which will simply initiaize mutex. > > > > Reported-and-tested-by: syzbot+5ca0bf339f13c4243001@syzkaller.appspotmail.com > > Fixes: 8572211842af ("[media] mxl111sf: convert to new DVB USB") > > Signed-off-by: Pavel Skripkin <paskripkin@gmail.com> > > Hi, Sean! > > Did you have a chance to review this patch? Thank you :) Sorry during the merge window (from -rc6 to -rc1) I don't tend to look at patches. Looks good to me, I'll merge it. Thanks Sean
diff --git a/drivers/media/usb/dvb-usb-v2/mxl111sf.c b/drivers/media/usb/dvb-usb-v2/mxl111sf.c index 7865fa0a8295..cd5861a30b6f 100644 --- a/drivers/media/usb/dvb-usb-v2/mxl111sf.c +++ b/drivers/media/usb/dvb-usb-v2/mxl111sf.c @@ -931,8 +931,6 @@ static int mxl111sf_init(struct dvb_usb_device *d) .len = sizeof(eeprom), .buf = eeprom }, }; - mutex_init(&state->msg_lock); - ret = get_chip_info(state); if (mxl_fail(ret)) pr_err("failed to get chip info during probe"); @@ -1074,6 +1072,14 @@ static int mxl111sf_get_stream_config_dvbt(struct dvb_frontend *fe, return 0; } +static int mxl111sf_probe(struct dvb_usb_device *dev) +{ + struct mxl111sf_state *state = d_to_priv(dev); + + mutex_init(&state->msg_lock); + return 0; +} + static struct dvb_usb_device_properties mxl111sf_props_dvbt = { .driver_name = KBUILD_MODNAME, .owner = THIS_MODULE, @@ -1083,6 +1089,7 @@ static struct dvb_usb_device_properties mxl111sf_props_dvbt = { .generic_bulk_ctrl_endpoint = 0x02, .generic_bulk_ctrl_endpoint_response = 0x81, + .probe = mxl111sf_probe, .i2c_algo = &mxl111sf_i2c_algo, .frontend_attach = mxl111sf_frontend_attach_dvbt, .tuner_attach = mxl111sf_attach_tuner, @@ -1124,6 +1131,7 @@ static struct dvb_usb_device_properties mxl111sf_props_atsc = { .generic_bulk_ctrl_endpoint = 0x02, .generic_bulk_ctrl_endpoint_response = 0x81, + .probe = mxl111sf_probe, .i2c_algo = &mxl111sf_i2c_algo, .frontend_attach = mxl111sf_frontend_attach_atsc, .tuner_attach = mxl111sf_attach_tuner, @@ -1165,6 +1173,7 @@ static struct dvb_usb_device_properties mxl111sf_props_mh = { .generic_bulk_ctrl_endpoint = 0x02, .generic_bulk_ctrl_endpoint_response = 0x81, + .probe = mxl111sf_probe, .i2c_algo = &mxl111sf_i2c_algo, .frontend_attach = mxl111sf_frontend_attach_mh, .tuner_attach = mxl111sf_attach_tuner, @@ -1233,6 +1242,7 @@ static struct dvb_usb_device_properties mxl111sf_props_atsc_mh = { .generic_bulk_ctrl_endpoint = 0x02, .generic_bulk_ctrl_endpoint_response = 0x81, + .probe = mxl111sf_probe, .i2c_algo = &mxl111sf_i2c_algo, .frontend_attach = mxl111sf_frontend_attach_atsc_mh, .tuner_attach = mxl111sf_attach_tuner, @@ -1311,6 +1321,7 @@ static struct dvb_usb_device_properties mxl111sf_props_mercury = { .generic_bulk_ctrl_endpoint = 0x02, .generic_bulk_ctrl_endpoint_response = 0x81, + .probe = mxl111sf_probe, .i2c_algo = &mxl111sf_i2c_algo, .frontend_attach = mxl111sf_frontend_attach_mercury, .tuner_attach = mxl111sf_attach_tuner, @@ -1381,6 +1392,7 @@ static struct dvb_usb_device_properties mxl111sf_props_mercury_mh = { .generic_bulk_ctrl_endpoint = 0x02, .generic_bulk_ctrl_endpoint_response = 0x81, + .probe = mxl111sf_probe, .i2c_algo = &mxl111sf_i2c_algo, .frontend_attach = mxl111sf_frontend_attach_mercury_mh, .tuner_attach = mxl111sf_attach_tuner,
Syzbot reported, that mxl111sf_ctrl_msg() uses uninitialized mutex. The problem was in wrong mutex_init() location. Previous mutex_init(&state->msg_lock) call was in ->init() function, but dvb_usbv2_init() has this order of calls: dvb_usbv2_init() dvb_usbv2_adapter_init() dvb_usbv2_adapter_frontend_init() props->frontend_attach() props->init() Since mxl111sf_* devices call mxl111sf_ctrl_msg() in ->frontend_attach() internally we need to initialize state->msg_lock before frontend_attach(). To achieve it, ->probe() call added to all mxl111sf_* devices, which will simply initiaize mutex. Reported-and-tested-by: syzbot+5ca0bf339f13c4243001@syzkaller.appspotmail.com Fixes: 8572211842af ("[media] mxl111sf: convert to new DVB USB") Signed-off-by: Pavel Skripkin <paskripkin@gmail.com> --- Changes in v3: I forgot to remove mutex_init() call from ->init() Changes in v2: Addressed same bug in all mxl111sf_* devices by adding ->probe call --- drivers/media/usb/dvb-usb-v2/mxl111sf.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-)