linux-kernel - Re: [PATCH v3 00/11] Introduce Simple atomic counters

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 for Android: free password hash cracker in your pocket

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

Date:   Fri, 9 Oct 2020 21:37:46 +0200
From:   Peter Zijlstra <peterz@...radead.org>
To:     Shuah Khan <skhan@...uxfoundation.org>
Cc:     corbet@....net, keescook@...omium.org, gregkh@...uxfoundation.org,
        shuah@...nel.org, rafael@...nel.org, johannes@...solutions.net,
        lenb@...nel.org, james.morse@....com, tony.luck@...el.com,
        bp@...en8.de, arve@...roid.com, tkjos@...roid.com,
        maco@...roid.com, joel@...lfernandes.org, christian@...uner.io,
        hridya@...gle.com, surenb@...gle.com, minyard@....org,
        arnd@...db.de, mchehab@...nel.org, rric@...nel.org,
        linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-kselftest@...r.kernel.org, linux-acpi@...r.kernel.org,
        devel@...verdev.osuosl.org,
        openipmi-developer@...ts.sourceforge.net,
        linux-edac@...r.kernel.org, Will Deacon <will@...nel.org>
Subject: Re: [PATCH v3 00/11] Introduce Simple atomic counters

On Fri, Oct 09, 2020 at 09:55:55AM -0600, Shuah Khan wrote:
> This patch series is a result of discussion at the refcount_t BOF
> the Linux Plumbers Conference. In this discussion, we identified
> a need for looking closely and investigating atomic_t usages in
> the kernel when it is used strictly as a counter without it
> controlling object lifetimes and state changes.
> 
> There are a number of atomic_t usages in the kernel where atomic_t api
> is used strictly for counting and not for managing object lifetime. In
> some cases, atomic_t might not even be needed.

Then the right patch is to not user atomic_t in those cases.

> The purpose of these counters is to clearly differentiate atomic_t
> counters from atomic_t usages that guard object lifetimes, hence prone
> to overflow and underflow errors. It allows tools that scan for underflow
> and overflow on atomic_t usages to detect overflow and underflows to scan
> just the cases that are prone to errors.

Guarding lifetimes is what we got refcount_t for.

> Simple atomic counters api provides interfaces for simple atomic counters
> that just count, and don't guard resource lifetimes. The interfaces are
> built on top of atomic_t api, providing a smaller subset of atomic_t
> interfaces necessary to support simple counters.

To what actual purpose?!? AFACIT its pointless wrappery, it gets us
nothing.

> Counter wraps around to INT_MIN when it overflows and should not be used
> to guard resource lifetimes, device usage and open counts that control
> state changes, and pm states. Overflowing to INT_MIN is consistent with
> the atomic_t api, which it is built on top of.
>     
> Using counter_atomic* to guard lifetimes could lead to use-after free
> when it overflows and undefined behavior when used to manage state
> changes and device usage/open states.
> 
> This patch series introduces Simple atomic counters. Counter atomic ops
> leverage atomic_t and provide a sub-set of atomic_t ops.

Thanks for Cc'ing the atomic maintainers :/

NAK.