On Fri, Feb 10, 2017 at 11:41:20AM -0800, Dan Williams wrote:
On Fri, Feb 10, 2017 at 11:19 AM, Logan Gunthorpe
<logang(a)deltatee.com> wrote:
> I copied this code and per feedback from Greg Kroah-Hartman [1] the
> cdev's kobject's parent should not be set to the related device.
> This should have minor consequences but isn't doing what anyone
> expects it to.
>
> This patch then fixes device-dax so it doesn't make the same mistake.
>
> [1]
https://lkml.org/lkml/2017/2/10/370
>
> Signed-off-by: Logan Gunthorpe <logang(a)deltatee.com>
Thanks for following up with this fix, but this causes a
use-after-free regression:
general protection fault: 0000 [#1] SMP DEBUG_PAGEALLOC
[..]
Call Trace:
vsnprintf+0x2d7/0x500
snprintf+0x49/0x60
dev_vprintk_emit+0x68/0x230
? debug_lockdep_rcu_enabled+0x1d/0x20
? trace_hardirqs_off+0xd/0x10
? cmpxchg_double_slab.isra.70+0x15a/0x1c0
? __slab_free+0x134/0x290
dev_printk_emit+0x4e/0x70
__dynamic_dev_dbg+0xc8/0x110
? __lock_acquire+0x33d/0x1290
dax_dev_huge_fault+0xee/0x570 [dax]
__handle_mm_fault+0x5aa/0x10a0
handle_mm_fault+0x154/0x350
? handle_mm_fault+0x3c/0x350
__do_page_fault+0x26b/0x4c0
trace_do_page_fault+0x58/0x270
do_async_page_fault+0x1a/0xa0
async_page_fault+0x28/0x30
I added this reference explicitly so the parent struct device has the
correct lifetime after this feedback from Al.
https://lists.01.org/pipermail/linux-nvdimm/2016-August/006563.html
...so I'm wondering what the actual problem is with setting cdev->parent?
It shouldn't do anything at all. The kobject in a cdev isn't a
"normal"
kobject, it doesn't show up in sysfs, or anywhere else. It's used for
an internal representation to the cdev code (a kmap) to look up the
object to call when userspace opens the device node in a quick manner.
Now changing from initialize/add to just register, does do different
things, perhaps that is the issue here. Just try removing the
cdev->kobject parent stuff and see if that causes a problem or not.
thanks,
greg k-h