On Fri, 2019-08-02 at 11:40 +0200, Greg Kroah-Hartman wrote:
From: Luke Nowakowski-Krijger lnowakow@eng.ucsd.edu
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: syzbot+a4387f5b6b799f6becbf@syzkaller.appspotmail.com
Signed-off-by: Luke Nowakowski-Krijger lnowakow@eng.ucsd.edu Signed-off-by: Hans Verkuil hverkuil-cisco@xs4all.nl [hverkuil-cisco@xs4all.nl: cleaned up two small checkpatch.pl warnings] [hverkuil-cisco@xs4all.nl: prefix subject with driver name] Signed-off-by: Mauro Carvalho Chehab mchehab+samsung@kernel.org Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org
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) {
return -ENOMEM;kfree(radio);
- }
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;
}