Messages in this thread | | | Subject | Re: Misplaced driver_sysfs_remove in really_probe? | From | Mukesh Ojha <> | Date | Thu, 28 Mar 2019 16:08:44 +0530 |
| |
On 3/28/2019 3:36 PM, Jiri Slaby wrote: > Hi, > > since commit 1901fb2604fbcd53201f38725182ea807581159e > Author: Kay Sievers <kay.sievers@novell.com> > Date: Sat Oct 7 21:55:55 2006 +0200 > > Driver core: fix "driver" symlink timing > > driver_sysfs_remove seems to be misplaced in the fail path of > really_probe. When driver_sysfs_add fails (or anything which is > currently above it in dd.c -- be it pinctrl_bind_pins or > dev->bus->dma_configure), driver_sysfs_remove is called. Given > dev->driver is set, attempt to remove sysfs device and driver links is > performed, but it is supposed to fail, as the links do not exist yet. > > I am dealing with a Syzkaller WARNING from SLE15-SP1 (4.12) which > corresponds to the described scenario. Perhaps Syzkaller fault-injected > a kzalloc failure to pinctrl_bind_pins as I cannot reproduce the report > at all: >> WARNING: CPU: 1 PID: 2091 at ../fs/kernfs/dir.c:1481 > kernfs_remove_by_name_ns+0xfa/0x120 fs/kernfs/dir.c:1480 >> Supported: No, Unreleased kernel >> CPU: 1 PID: 2091 Comm: systemd-udevd Not tainted 4.12.14-396-default > #1 SLE15-SP1 (unreleased) >> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS > rel-1.12.0-0-ga698c89-prebuilt.qemu.org 04/01/2014 >> task: ffff880053794fc0 task.stack: ffff880040ea0000 >> RIP: 0010:kernfs_remove_by_name_ns+0xfa/0x120 fs/kernfs/dir.c:1480 >> RSP: 0018:ffff880040ea7488 EFLAGS: 00010282 >> RAX: 000000000000002d RBX: 0000000000000000 RCX: 0000000000000000 >> RDX: 000000000000002d RSI: 1ffff100081d4e48 RDI: ffffed00081d4e85 >> RBP: ffffffffa772a380 R08: 0000000000000000 R09: 0000000000000000 >> R10: ffffffffaa1872c4 R11: 0000000000000000 R12: 0000000000000000 >> R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000037 >> FS: 00007f0d24dc0f80(0000) GS:ffff88005e080000(0000) > knlGS:0000000000000000 >> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 >> CR2: 000000000045c061 CR3: 000000003b878006 CR4: 0000000000060ee0 >> Call Trace: >> driver_sysfs_remove+0xb0/0x110 drivers/base/dd.c:290 >> really_probe drivers/base/dd.c:433 [inline] >> driver_probe_device+0x2b3/0x1200 drivers/base/dd.c:530 >> __driver_attach+0x1dc/0x280 drivers/base/dd.c:763 >> bus_for_each_dev+0x146/0x1e0 drivers/base/bus.c:316 >> bus_add_driver+0x40f/0x850 drivers/base/bus.c:710 >> driver_register+0x1c9/0x410 drivers/base/driver.c:168 >> __hid_register_driver+0x1e0/0x2d0 drivers/hid/hid-core.c:2974 >> ? 0xffffffffc1510000 >> do_one_initcall+0xb7/0x300 init/main.c:808 >> do_init_module+0x23e/0x641 kernel/module.c:3515 >> load_module+0x47d6/0x60b0 kernel/module.c:3867 >> SYSC_finit_module+0x239/0x2a0 kernel/module.c:3980 >> do_syscall_64+0x26c/0x6e0 arch/x86/entry/common.c:284 >> entry_SYSCALL_64_after_hwframe+0x3d/0xa2 > I believe it survived from 2.6.20 to the current tree. > > Does it look correct to you? This should help IMO and if you agree I > will send a proper patch: > --- a/drivers/base/dd.c > +++ b/drivers/base/dd.c > @@ -496,7 +496,7 @@ static int really_probe(struct device *dev, struct > device_driver *drv) > if (driver_sysfs_add(dev)) { > printk(KERN_ERR "%s: driver_sysfs_add(%s) failed\n", > __func__, dev_name(dev)); > - goto probe_failed; > + goto sysfs_failed; > } > > if (dev->pm_domain && dev->pm_domain->activate) { > @@ -546,6 +546,8 @@ static int really_probe(struct device *dev, struct > device_driver *drv) > goto done; > > probe_failed: > + driver_sysfs_remove(dev); > +sysfs_failed: > arch_teardown_dma_ops(dev); > dma_failed: > if (dev->bus) > @@ -554,7 +556,6 @@ static int really_probe(struct device *dev, struct > device_driver *drv) > pinctrl_bind_failed: > device_links_no_driver(dev); > devres_release_all(dev); > - driver_sysfs_remove(dev); > dev->driver = NULL; > dev_set_drvdata(dev, NULL); > if (dev->pm_domain && dev->pm_domain->dismiss)
Nice observation. Please send a proper patch.
Cheers, Mukesh
> > > > > thanks,
| |