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: syzbot+c063a4e176681d2e0380@xxxxxxxxxxxxxxxxxxxxxxxxx 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
