On Mon, Mar 22, 2021 at 08:47:17PM -0700, Lv Yunlong wrote: > In usbtmc_disconnect, data is got from intf with the > initial reference. There is no refcount inc operation > before usbmc_free_int(data). In usbmc_free_int(data), > the data may be freed. > > But later in usbtmc_disconnect, there is another put > function of data. I think it is better to add necessary > checks to avoid the data being put twice. It could cause > errors in race. > > Signed-off-by: Lv Yunlong <lyl2...@mail.ustc.edu.cn> > --- > drivers/usb/class/usbtmc.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c > index 74d5a9c5238a..e0438cb46386 100644 > --- a/drivers/usb/class/usbtmc.c > +++ b/drivers/usb/class/usbtmc.c > @@ -2494,7 +2494,9 @@ static void usbtmc_disconnect(struct usb_interface > *intf) > } > mutex_unlock(&data->io_mutex); > usbtmc_free_int(data); > - kref_put(&data->kref, usbtmc_delete); > + > + if (data->iin_ep_present && data->iin_urb) > + kref_put(&data->kref, usbtmc_delete);
What protects the data from changing right after the check and right before the kref_put() call? krefs need a lock somewhere to protect from races like this, please fix that logic instead. thanks, greg k-h
