On Fri, Sep 08, 2023 at 09:09:37PM +0530, Yuran Pereira wrote:
>
> When the call to dev_set_name() in the usb_hub_create_port_device
> function fails to set the device's kobject's name field,
> the subsequent call to device_register() is bound to fail and cause
> a NULL pointer derefference, because kobject_add(), which is in the
> call sequence, expects the name field to be set before it is called
>
>
> This patch adds code to perform error checking for dev_set_name()'s
> return value. If the call to dev_set_name() was unsuccessful,
> usb_hub_create_port_device() returns with an error.
>
>
> PS: The patch also frees port_dev->req and port_dev before returning.
> However, I am not sure if that is necessary, because port_dev->req
> and port_dev are not freed when device_register() fails. Would be
> happy if someone could help me understand why that is, and whether I
> should keep those kfree calls in my patch.
>
> dashboard link: https://syzkaller.appspot.com/bug?extid=c063a4e176681d2e0380
>
> Reported-by: [email protected]
It shouldn't be necessary to check the return value from dev_set_name().
Most of its other call sites ignore the return value. In fact, only one
of the call sites in drivers/base/core.c does check the return value!
Furthermore, device_add() includes the following test for whether the
device's name has been set:
if (!dev_name(dev)) {
error = -EINVAL;
goto name_error;
}
The real question is why this test did not prevent the bug from
occurring. Until you can answer that question, any fix you propose is
highly questionable.
Alan Stern