Hi Takashi,

On Fri, Jan 22, 2021 at 04:47:44PM +0100, Robert Foss wrote:

Hey Takashi,

This patch is generating a checkpatch warning, but I think it is
spurious and can be ignored.

The checkpatch warning isn't superious and should really be corrected.

Other than that, this looks good to me.
Reviewed-by: Robert Foss <redacted>

On Wed, 20 Jan 2021 at 12:51, Takashi Iwai [off-list ref] wrote:

quoted

dvb_usb_device_init() copies the properties to the own data, so that
the callers can release the original properties later (as done in the
commit 299c7007e936 "media: dw2102: Fix memleak on sequence of
probes").  However, it also stores dev->desc pointer that is a
reference to the original properties data.  Since dev->desc is
referred later, it may result in use-after-free, in the worst case,
leading to a kernel Oops as reported.

This patch addresses the problem by allocating and copying the
properties at first, then get the desc from the copied properties.

Reported-and-tested-by: Stefan Seyfried <redacted>
BugLink: http://bugzilla.opensuse.org/show_bug.cgi?id=1181104
Cc: <redacted>
Signed-off-by: Takashi Iwai <redacted>

Thank you for your patch. Unfortunately, it depends on the first patch
in the series, which I think has problems (see email about this).

Thanks

Sean

quoted

---
 drivers/media/usb/dvb-usb/dvb-usb-init.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/drivers/media/usb/dvb-usb/dvb-usb-init.c b/drivers/media/usb/dvb-usb/dvb-usb-init.c
index 5befec87f26a..07ff9b4d2f34 100644
--- a/drivers/media/usb/dvb-usb/dvb-usb-init.c
+++ b/drivers/media/usb/dvb-usb/dvb-usb-init.c

@@ -255,27 +255,30 @@ int dvb_usb_device_init(struct usb_interface *intf,
        if (du != NULL)
                *du = NULL;

-       if ((desc = dvb_usb_find_device(udev, props, &cold)) == NULL) {
+       d = kzalloc(sizeof(struct dvb_usb_device), GFP_KERNEL);
+       if (!d) {
+               err("no memory for 'struct dvb_usb_device'");
+               return -ENOMEM;
+       }
+
+       memcpy(&d->props, props, sizeof(struct dvb_usb_device_properties));
+
+       desc = dvb_usb_find_device(udev, &d->props, &cold);
+       if (!desc) {
                deb_err("something went very wrong, device was not found in current device list - let's see what comes next.\n");
-               return -ENODEV;
+               ret = -ENODEV;
+               goto error;
        }

        if (cold) {
                info("found a '%s' in cold state, will try to load a firmware", desc->name);
                ret = dvb_usb_download_firmware(udev, props);
                if (!props->no_reconnect || ret != 0)
-                       return ret;
+                       goto error;
        }

        info("found a '%s' in warm state.", desc->name);
-       d = kzalloc(sizeof(struct dvb_usb_device), GFP_KERNEL);
-       if (d == NULL) {
-               err("no memory for 'struct dvb_usb_device'");
-               return -ENOMEM;
-       }
-
        d->udev = udev;
-       memcpy(&d->props, props, sizeof(struct dvb_usb_device_properties));
        d->desc = desc;
        d->owner = owner;

--

2.26.2