Re: [PATCH 3/3] creds: convert cred.usage to refcount_t

[Xiaoming Ni]

On 2020/6/13 2:34, Kees Cook wrote:
> From: Elena Reshetova <elena.reshetova@xxxxxxxxx>
>
> atomic_t variables are currently used to implement reference
> counters with the following properties:
>   - counter is initialized to 1 using atomic_set()
>   - a resource is freed upon counter reaching zero
>   - once counter reaches zero, its further
>     increments aren't allowed
>   - counter schema uses basic atomic operations
>     (set, inc, inc_not_zero, dec_and_test, etc.)
>
> Such atomic variables should be converted to a newly provided
> refcount_t type and API that prevents accidental counter overflows
> and underflows. This is important since overflows and underflows
> can lead to use-after-free situation and be exploitable.
>
> The variable cred.usage is used as pure reference counter.
> Convert it to refcount_t and fix up the operations.
>
> **Important note for maintainers:
>
> Some functions from refcount_t API defined in lib/refcount.c
> have different memory ordering guarantees than their atomic
> counterparts.Please check Documentation/core-api/refcount-vs-atomic.rst
> for more information.
>
> Normally the differences should not matter since refcount_t provides
> enough guarantees to satisfy the refcounting use cases, but in
> some rare cases it might matter.
> Please double check that you don't have some undocumented
> memory guarantees for this variable usage.
>
> For the cred.usage it might make a difference
> in following places:
>   - get_task_cred(): increment in refcount_inc_not_zero() only
>     guarantees control dependency on success vs. fully ordered
>     atomic counterpart
>   - put_cred(): decrement in refcount_dec_and_test() only
>     provides RELEASE ordering and ACQUIRE ordering on success
>     vs. fully ordered atomic counterpart
>
> Suggested-by: Kees Cook <keescook@xxxxxxxxxxxx>
> Signed-off-by: Elena Reshetova <elena.reshetova@xxxxxxxxx>
> Reviewed-by: David Windsor <dwindsor@xxxxxxxxx>
> Reviewed-by: Hans Liljestrand <ishkamiel@xxxxxxxxx>
> Link: https://lore.kernel.org/r/20190306110549.7628-4-elena.reshetova@xxxxxxxxx
> Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx>

Currently this patch is better than my RFC patch
Looks good to me.

Thanks
Xiaoming Ni