linux-kernel - Re: [PATCH 2/5] kernfs: make ->attr.open RCU protected.

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] [day] [month] [year] [list]

Message-ID: <YoMitbmf5hIm7QRh@slm.duckdns.org>
Date:   Mon, 16 May 2022 18:21:09 -1000
From:   Tejun Heo <tj@...nel.org>
To:     Imran Khan <imran.f.khan@...cle.com>
Cc:     viro@...iv.linux.org.uk, gregkh@...uxfoundation.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/5] kernfs: make ->attr.open RCU protected.

On Tue, May 17, 2022 at 12:30:26PM +1000, Imran Khan wrote:
> +/**
> + *     kernfs_deref_open_node - Get kernfs_open_node corresponding to @kn.
> + *
> + *     @kn: target kernfs_node.
> + *     @val: RCU dereference will take place only if val is true.
> + *
> + *     Fetch and return ->attr.open of @kn when current updater of ->attr.open
> + *     ensures that as long as @val is true, other updater(s) of ->attr.open
> + *     can not change it. The caller needs to pass value of the condition
> + *     (@val) that prevents value of ->attr.open from changing.
> + *
> + *     This should ONLY be used by updaters of ->attr.open.
> + */
> +static struct kernfs_open_node *
> +kernfs_deref_open_node(struct kernfs_node *kn, bool val)
> +{
> +       return rcu_dereference_protected(kn->attr.open, val);
> +}
> +
> +/**
> + *     kernfs_check_open_node - Get kernfs_open_node corresponding to @kn.
> + *
> + *     @kn: target kernfs_node.
> + *     @val: RCU dereference will take place only if val is true.
> + *
> + *     rcu_dereference and return ->attr.open of @kn. This is used in cases
> + *     where we can access ->attr.open outside RCU read-side critical section
> + *     as long as specified conditions are correct i.e @val is true.
> + *
> + *     This should ONLY be used by readers of ->attr.open.
> + */
> +static struct kernfs_open_node *
> +kernfs_check_open_node(struct kernfs_node *kn, bool val)
> +{
> +       return rcu_dereference_check(kn->attr.open, val);
> +}

You gotta put the conditions inside these wrappers. These wrappers are
supposed to describe as strict as reasonably possible so that the checks can
catch people making mistakes. It doesn't make much sense to defer the
condition to the callers. Just describe what the expected conditions are in
the wrappers as best as you reasonably can.

Thanks.

-- 
tejun