On Tue, 2018-07-10 at 16:55 -0700, Linus Torvalds wrote: > On Tue, Jul 10, 2018 at 4:32 PM Benjamin Herrenschmidt > <b...@kernel.crashing.org> wrote: > > > > > I like that fix, which should make this patch obsolete, right? > > > > Yes, for that specific issue, but Linus seemed to think patch 1 was the > > "right thing to do" regardless... > > I would definitely prefer either a kobject_get_unless_zero() or a > warning if it is ever zero. > > The fact that right now it silently can do known bad things, and then > causes odd corruption _later_, is not good.
Maybe we should make the existing warning in refcount unconditional ? This is whe warning that allowed me to pull that string with the gluedirs and fix what ended up very weird crashes caused by the resulting use-after-free. So it's definitely valuable. As for whether we should generalize kobject_get_unless_zero() vs avoid using it alltogether, this is a debate for you to have with Greg ;-) He seems to think nothing should ever try to get a zeroed object (which I tend to agree with, it's close to my opinion that visibility and lifetime should be disconnected). That being said, there are existing constructs such as the "late removal from sysfs from kobject_release" that mean that zero-reference objects *can* still be visible, via either sysfs or ksets, as far as I can tell. So it's a bit of a mess... but if we chose to go Greg's way we should probably put a WARN'ing in kobject_release() for late-removal from sysfs since this exposes 0-ref objects to outside visibility. Cheers, Ben.