* Misplaced driver_sysfs_remove in really_probe?
@ 2019-03-28 10:06 Jiri Slaby
2019-03-28 10:38 ` Mukesh Ojha
0 siblings, 1 reply; 2+ messages in thread
From: Jiri Slaby @ 2019-03-28 10:06 UTC (permalink / raw)
To: Greg KH, rafael, Linux kernel mailing list
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)
thanks,
--
js
suse labs
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: Misplaced driver_sysfs_remove in really_probe?
2019-03-28 10:06 Misplaced driver_sysfs_remove in really_probe? Jiri Slaby
@ 2019-03-28 10:38 ` Mukesh Ojha
0 siblings, 0 replies; 2+ messages in thread
From: Mukesh Ojha @ 2019-03-28 10:38 UTC (permalink / raw)
To: Jiri Slaby, Greg KH, rafael, Linux kernel mailing list
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,
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2019-03-28 10:39 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-28 10:06 Misplaced driver_sysfs_remove in really_probe? Jiri Slaby
2019-03-28 10:38 ` Mukesh Ojha
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Powered by JetHome