linux-kernel - Re: [PATCH 2/2] drivers: core: Remove glue dirs from sysfs earlier

lists.openwall.net		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 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 2 Jul 2018 19:15:33 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        Tejun Heo <tj@...nel.org>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "Eric W. Biederman" <ebiederm@...ssion.com>,
        Joel Stanley <joel@....id.au>
Subject: Re: [PATCH 2/2] drivers: core: Remove glue dirs from sysfs earlier

On Mon, Jul 2, 2018 at 5:57 PM Benjamin Herrenschmidt
<benh@...nel.crashing.org> wrote:
>
> I'll have a look see if I can find a way to check that the sysfs dir is
> empty without adding that childcount, that would at least alleviate
> some of your objection, and would probably make the patch
> smaller/simpler.

That would definitely make me happier.

Right now we already remove the actual device node sysfs associations
synchronously with "kobject_del()" (even if it still then stays around
as a kobject), and that does the remove for the object itself - just
not the glue directory.

If we then just did a "if glue dir is empty, delete the glue dir too"
in cleanup_glue_dir(), at least ther patch would be prettier.

I *think* that should be easy. Maybe. Something almost, but not quite,
entirely unlike the below UNTESTED hack?

It's whitespace-damaged on purpose. It's probably broken shit. DO NOT
USE UNDER ANY CIRCUMSTANCES.  Think of it more as a "something like
this might work, but probably doesn't". Maybe it gives you an idea,
although that idea might be "Linus has finally lost it".

I suspect it's broken because kernfs has this notion of inactive
nodes, so just testing for the rbtree being empty is probably wrong. I
think maybe it needs to test for there not being any active nodes, but
then you'd need the kernfs_mutex etc, so that would make things much
worse.

I really haven't touched the kernfs code much, thus the "this may be
complete and utter garbage" warning.

Adding Tejun to the participants, he should know. Or Greg, who has
been silent, presumably because he's still getting over his crazy
travels.

                Linus

---

    --- a/drivers/base/core.c
   +++ b/drivers/base/core.c
   @@ -1585,6 +1585,12 @@ static inline struct kobject
*get_glue_dir(struct device *dev)
        return dev->kobj.parent;
    }

   +static inline bool has_children(struct kobject *dir)
   +{
   +    struct kernfs_node *kn = dir->sd;
   +    return kn && kn->dir.children.rb_node;
   +}
   +
    /*
     * make sure cleaning up dir as the last step, we need to make
     * sure .release handler of kobject is run with holding the
   @@ -1597,6 +1603,10 @@ static void cleanup_glue_dir(struct device
*dev, struct kobject *glue_dir)
                return;

        mutex_lock(&gdp_mutex);
   -    kobject_put(glue_dir);
   +    if (has_children(glue_dir))
   +            kobject_put(glue_dir);
   +    else
   +            kobject_del(glue_dir);
        mutex_unlock(&gdp_mutex);
    }