On Fri, 2019-08-02 at 11:40 +0200, Greg Kroah-Hartman wrote: > From: Luke Nowakowski-Krijger <[email protected]> > > commit c666355e60ddb4748ead3bdd983e3f7f2224aaf0 upstream. > > Change devm_k*alloc to k*alloc to manually allocate memory > > The manual allocation and freeing of memory is necessary because when > the USB radio is disconnected, the memory associated with devm_k*alloc > is freed. Meaning if we still have unresolved references to the radio > device, then we get use-after-free errors. > > This patch fixes this by manually allocating memory, and freeing it in > the v4l2.release callback that gets called when the last radio device > exits.
This really should be commented in the code and not just in the commit changelog as some unsuspecting person will likely undo this in the future without one. > Reported-and-tested-by: [email protected] > > Signed-off-by: Luke Nowakowski-Krijger <[email protected]> > Signed-off-by: Hans Verkuil <[email protected]> > [[email protected]: cleaned up two small checkpatch.pl warnings] > [[email protected]: prefix subject with driver name] > Signed-off-by: Mauro Carvalho Chehab <[email protected]> > Signed-off-by: Greg Kroah-Hartman <[email protected]> > > --- > drivers/media/radio/radio-raremono.c | 30 +++++++++++++++++++++++------- > 1 file changed, 23 insertions(+), 7 deletions(-) > > --- a/drivers/media/radio/radio-raremono.c > +++ b/drivers/media/radio/radio-raremono.c > @@ -271,6 +271,14 @@ static int vidioc_g_frequency(struct fil > return 0; > } > > +static void raremono_device_release(struct v4l2_device *v4l2_dev) > +{ > + struct raremono_device *radio = to_raremono_dev(v4l2_dev); > + > + kfree(radio->buffer); > + kfree(radio); > +} > + > /* File system interface */ > static const struct v4l2_file_operations usb_raremono_fops = { > .owner = THIS_MODULE, > @@ -295,12 +303,14 @@ static int usb_raremono_probe(struct usb > struct raremono_device *radio; > int retval = 0; > > - radio = devm_kzalloc(&intf->dev, sizeof(struct raremono_device), > GFP_KERNEL); > - if (radio) > - radio->buffer = devm_kmalloc(&intf->dev, BUFFER_LENGTH, > GFP_KERNEL); > - > - if (!radio || !radio->buffer) > + radio = kzalloc(sizeof(*radio), GFP_KERNEL); > + if (!radio) > + return -ENOMEM; > + radio->buffer = kmalloc(BUFFER_LENGTH, GFP_KERNEL); > + if (!radio->buffer) { > + kfree(radio); > return -ENOMEM; > + } > > radio->usbdev = interface_to_usbdev(intf); > radio->intf = intf; > @@ -324,7 +334,8 @@ static int usb_raremono_probe(struct usb > if (retval != 3 || > (get_unaligned_be16(&radio->buffer[1]) & 0xfff) == 0x0242) { > dev_info(&intf->dev, "this is not Thanko's Raremono.\n"); > - return -ENODEV; > + retval = -ENODEV; > + goto free_mem; > } > > dev_info(&intf->dev, "Thanko's Raremono connected: (%04X:%04X)\n", > @@ -333,7 +344,7 @@ static int usb_raremono_probe(struct usb > retval = v4l2_device_register(&intf->dev, &radio->v4l2_dev); > if (retval < 0) { > dev_err(&intf->dev, "couldn't register v4l2_device\n"); > - return retval; > + goto free_mem; > } > > mutex_init(&radio->lock); > @@ -345,6 +356,7 @@ static int usb_raremono_probe(struct usb > radio->vdev.ioctl_ops = &usb_raremono_ioctl_ops; > radio->vdev.lock = &radio->lock; > radio->vdev.release = video_device_release_empty; > + radio->v4l2_dev.release = raremono_device_release; > > usb_set_intfdata(intf, &radio->v4l2_dev); > > @@ -360,6 +372,10 @@ static int usb_raremono_probe(struct usb > } > dev_err(&intf->dev, "could not register video device\n"); > v4l2_device_unregister(&radio->v4l2_dev); > + > +free_mem: > + kfree(radio->buffer); > + kfree(radio); > return retval; > } > > >
