Re: [PATCH] cred: clarify usage of get_cred_rcu()

[Paul Moore]

On Fri, Feb 20, 2026 at 4:19 AM Alice Ryhl <aliceryhl@xxxxxxxxxx> wrote:
>
> After being confused by looking at get_cred() and get_cred_rcu(), I
> figured out what's going on. Thus, add some comments to clarify how
> get_cred_rcu() works for the benefit of others looking in the future.
>
> Note that in principle we could add an assertion that non_rcu is zero in
> the failure path of atomic_long_inc_not_zero().

That would be interesting to add a WARN_ON() there and see what
happens.  Hopefully nothing, but one never knows ;)  Have you tried
this?

> Signed-off-by: Alice Ryhl <aliceryhl@xxxxxxxxxx>
> ---
>  include/linux/cred.h | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)

...

> +/*
> + * get_cred_rcu - Get a reference on a set of credentials under rcu

I agree this is a bit pedantic, but it looks like the bulk of the file
capitalizes RCU and technically that is correct as it is an acronym.

> + * @cred: The credentials to reference
> + *
> + * Get a reference on the specified set of credentials, or %NULL if the last
> + * refcount has already been put.
> + *
> + * This is used to obtain a reference under an rcu read lock.

I would suggest a different description:

"Get a reference to the specified set of credentials and return a
pointer to the cred struct, or %NULL if it is not possible to obtain a
new reference.  After successfully taking a new reference to the
specified credentials, the cred struct will be marked for free'ing via
RCU."

> + */
>  static inline const struct cred *get_cred_rcu(const struct cred *cred)
>  {
>         struct cred *nonconst_cred = (struct cred *) cred;
>         if (!cred)
>                 return NULL;
>         if (!atomic_long_inc_not_zero(&nonconst_cred->usage))
>                 return NULL;
> +       /*
> +        * If non_rcu is not already zero, then this call to get_cred_rcu() is
> +        * probably wrong because if 'usage' goes to zero prior to this call,
> +        * then get_cred_rcu() assumes it is freed with rcu.
> +        *
> +        * However, an exception to this is using get_cred_rcu() in cases where
> +        * get_cred() would have been okay. To support that case, we do not
> +        * check non_rcu and set it to zero regardless.
> +        */

This is surely a matter of perspective, but the above seems a bit
wordy, and doesn't address what I believe is the important part:
setting non_rcu to zero means this credential will be freed
asynchronously via RCU.  Both get_cred_rcu() and get_cred() set
non_rcu to 0/false ... although get_cred() doesn't do the non-zero
check before bumping the refcount.

I suppose we could consider adding the zero check in the get_cred()
case, but even if we ignore the KCSAN barrier, it looks like the arch
support for the inc_not_zero() case isn't nearly as good, likely
resulting in more code to execute.

>         nonconst_cred->non_rcu = 0;
>         return cred;
>  }
> --
> 2.53.0.345.g96ddfc5eaa-goog

-- 
paul-moore.com