Re: kobject_init_and_add is easy to misuse

[Jason Gunthorpe <jgg@ziepe.ca>]

On Wed, Jun 03, 2020 at 01:56:20PM -0700, James Bottomley wrote:

> The store of state_in_sysfs is already done in kobject_add_internal(). 
> It's an existing flag people already use to tell if the kobject has
> been exposed in sysfs.  However, it's set after the sysfs directory
> creation succeeds.  This is the code with some debugging removed:
> 
> 	error = create_dir(kobj);
> 	if (error) {
> 		kobj_kset_leave(kobj);
> 		kobject_put(parent);
> 		kobj->parent = NULL;
> 		...
> 	} else
> 		kobj->state_in_sysfs = 1;

I was thinking most probably this will need a lock or a
smp_store_release() ..

> > It feels more robust to stick with the put though..
> 
> possibly ... like I said, the only concern with the put path is that
> ->release has state expectations that aren't met if
> kobject_init_and_add fails.

Certainly error unwind bugs related to put and release will exist, but
I suspect switching to kfree won't solve them, just move them to the
next function that fails and needs a put based unwind?

At least the patches I reviewed for RDM a from Wang Hai were all
correct and didn't seem to have release based errors.

Jason