Re: [PATCH 15/15] vfio/mdev: remove an extra parent kobject reference

Jason Gunthorpe
2022-07-06T14:38:38Z

On Wed, Jul 06, 2022 at 09:42:19AM +0200, Christoph Hellwig wrote:
> The mdev_type already holds a reference to the parent through
> mdev_types_kset, so drop the extra reference.

I would drop this patch, but at least the explanation needs tweaking..

kobj's are weird things, they have a kobj.parent, but the refcount for
that is dropped during kobject_del() and parent is NULL'd. vs this
reference which is being put back at kobject release.

So, the extra kset reference isn't contributing more protection - when
mdev_unregister_device() calls kset_unregister() it still has a valid
reference on the parent from the caller.

If this change is safe it is because no accesses to mdev->parent are
happening after mdev_unregister_device() returns, due to things like
the sysfs attribute fencing and the mdev_unreg_sem fencing child
devices. I didn't check everything, but it seems believable it could
be true.

I also suggest this patch should NULL mdev_type->parent during
unregister_device as the kobj stuff does, so any use after unregister
but before release could be crash, we generally shouldn't leave
unref'd pointers floating around.

Jason