Re: [PATCH] device-dax: don't set kobj parent during cdev init

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

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@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@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