Hello, Linus. On Sat, May 20, 2017 at 01:03:14AM +0200, Linus Walleij wrote: > The ATA hosts are allocated using devres with: > host = devres_alloc(ata_host_release, sz, GFP_KERNEL); > However in the ata_host_release() function the host is retrieved > using dev_get_drvdata() which is not what other devres handlers > do, instead we should probably use the passed resource. > > Before this my kernel crashes badly when I fail to start a host > in ata_host_start() and need to bail out, because dev_get_drvdata() > gets the wrong-but-almost-correct pointer (so on some systems it > may by chance be the right pointer what do I know). > > On ARMv4 Gemini it is not: > > ------------[ cut here ]------------ > WARNING: CPU: 0 PID: 1 at ../lib/refcount.c:184 refcount_sub_and_test+0x9c/0xac > refcount_t: underflow; use-after-free. > CPU: 0 PID: 1 Comm: swapper Not tainted 4.12.0-rc1+ #657 > Hardware name: Gemini (Device Tree) > [<c0010f10>] (unwind_backtrace) from [<c000d8a4>] (show_stack+0x10/0x14) > [<c000d8a4>] (show_stack) from [<c0018720>] (__warn+0xcc/0xf4) > [<c0018720>] (__warn) from [<c0018780>] (warn_slowpath_fmt+0x38/0x48) > [<c0018780>] (warn_slowpath_fmt) from [<c01fffcc>] (refcount_sub_and_test+0x9c/0xac) > [<c01fffcc>] (refcount_sub_and_test) from [<c01e8a5c>] (kobject_put+0x28/0xe0) > [<c01e8a5c>] (kobject_put) from [<c029b294>] (ata_host_release+0xb0/0x144) > [<c029b294>] (ata_host_release) from [<c027326c>] (release_nodes+0x178/0x1fc) > [<c027326c>] (release_nodes) from [<c02707e4>] (driver_probe_device+0xd0/0x2dc) > [<c02707e4>] (driver_probe_device) from [<c0270aac>] (__driver_attach+0xbc/0xc0) > [<c0270aac>] (__driver_attach) from [<c026eeac>] (bus_for_each_dev+0x70/0xa0) > [<c026eeac>] (bus_for_each_dev) from [<c026f824>] (bus_add_driver+0x178/0x200) > [<c026f824>] (bus_add_driver) from [<c0271184>] (driver_register+0x78/0xf8) > [<c0271184>] (driver_register) from [<c05b2d90>] (do_one_initcall+0xac/0x174) > [<c05b2d90>] (do_one_initcall) from [<c05b2f6c>] (kernel_init_freeable+0x114/0x1cc) > [<c05b2f6c>] (kernel_init_freeable) from [<c04beeb4>] (kernel_init+0x8/0xf4) > [<c04beeb4>] (kernel_init) from [<c000a270>] (ret_from_fork+0x14/0x24) > ---[ end trace 0a4570446a019085 ]--- > > Then there is a second (worse) crash when it tries to iterate > to the next port. But it is all because the host pointer is > wrong. This is really weird. The two can't be different, well, at least shouldn't. > In this case, the host should be 0xc7a3f3d0 as it was when it got > allocated but instead what dev_get_drvdata() returns is 0xc7a3f370. > Using the passed resource gives the right pointer. That's 96 bytes of difference, which seems too big for devres_node, especially on 32bit machines. Can you check what gdb says on "print ((struct devres *)0)->data" or "print sizeof(struct devres_node)"? There gotta be something else going on. devres_alloc() returns the data pointer which is the same one which gets passed into the release function. Thanks. -- tejun