lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  PHC 
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:   Sat, 11 May 2019 02:32:19 -0400
From:   "Tobin C. Harding" <>
To:     "Petr Mladek" <>
Cc:     "Rafael J. Wysocki" <>,
        "Greg Kroah-Hartman" <>,
        "Tyrel Datwyler" <>,
        "Andy Shevchenko" <>,
        "Miroslav Benes" <>,
        "Viresh Kumar" <>,
        "Linux Kernel Mailing List" <>
Subject: Re: kobject_init_and_add() confusion

On Fri, May 10, 2019, at 19:40, Petr Mladek wrote:
> On Fri 2019-05-10 12:35:38, Tobin C. Harding wrote:
> > On Wed, May 01, 2019 at 09:54:16AM +0200, Rafael J. Wysocki wrote:
> > > On Wed, May 1, 2019 at 1:38 AM Tobin C. Harding <> wrote:
> > > > TODO
> > > > ----
> > > >
> > > > - Fix all the callsites to kobject_init_and_add()
> > > > - Further clarify the function docstring for kobject_init_and_add() [perhaps]
> > > > - Add a section to Documentation/kobject.txt [optional]
> > > > - Add a sample usage file under samples/kobject [optional]
> > > 
> > > The plan sounds good to me, but there is one thing to note IMO:
> > > kobject_cleanup() invokes the ->release() callback for the ktype, so
> > > these callbacks need to be able to cope with kobjects after a failing
> > > kobject_add() which may not be entirely obvious to developers
> > > introducing them ATM.
> > 
> > It has taken a while for this to soak in.  This is actually quite an
> > insidious issue.  If I give an example and perhaps we can come to a
> > solution.  This example is based on the code (and assumptions) in
> > mm/slub.c
> > 
> > If a developer has an object that they wish to add to sysfs they go
> > ahead and embed a kobject in it.  Correctly set up a ktype including
> > release function that just frees the object (using container of).  Now
> > assume that the object is already set up and in use when we go to set up
> > the sysfs entry.
> It would say that this is a bad design. I see the creation of the sysfs
> entry as part of the initialization. The object should not be made
> usable before it is fully initialized.

It may be a case of my lack of understanding of object lifecycles here and not bad design, if as you say creation of sysfs is always part of initialisation then the problem I describe above should not exist (and it may well not, assumptions behind code are hard to grok).
> > If kobject_init_and_add() fails and we correctly call
> > kobject_put() the containing object will be free'd.  Yet the calling
> > code may not be done with the object, more to the point just because
> > sysfs setup fails the object is now unusable.  Besides the interesting
> > theoretical discussion this means we cannot just go and willy-nilly add
> > calls to kobject_put() in the error path of kobject_init_and_add() if
> > the original code was not written under the assumption that the release
> > method could be called during the error path (I have found 2 places at
> > least where behaviour of calling the release method is non-trivial to
> > ascertain).
> kobject usage is complicated and it is easy to make it wrong. I think
> that this is motivation to improve the documentation and adding
> good examples.

Cool, I did work on adding your example from last week into samples/kobject but I wasn't able to come up with anything that I was totally happy with.  Hard to use API needs minimal concise correct examples right, I'm going to keep at that as I learn more from seeing/patching current kobject code.

> > I guess, as Greg said, its just a matter that reference counting within
> > the kernel is a hard problem.  So we fix the easy ones and then look a
> > bit harder at the hard ones ...
> The people working on the affected subsystem should be able to help.
> They might have misunderstood kobjects. But they should be more
> familiar with the other dependencies.

Sure thing.

> Thanks for working on it.

Things that bend ones brain are the funnest to work on ;)


Powered by blists - more mailing lists