linux-acpi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Shuah Khan <skhan@linuxfoundation.org>
Cc: corbet@lwn.net, keescook@chromium.org,
	gregkh@linuxfoundation.org, shuah@kernel.org, rafael@kernel.org,
	johannes@sipsolutions.net, lenb@kernel.org, james.morse@arm.com,
	tony.luck@intel.com, bp@alien8.de, arve@android.com,
	tkjos@android.com, maco@android.com, joel@joelfernandes.org,
	christian@brauner.io, hridya@google.com, surenb@google.com,
	minyard@acm.org, arnd@arndb.de, mchehab@kernel.org,
	rric@kernel.org, linux-doc@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org,
	linux-acpi@vger.kernel.org, devel@driverdev.osuosl.org,
	openipmi-developer@lists.sourceforge.net,
	linux-edac@vger.kernel.org, Will Deacon <will@kernel.org>
Subject: Re: [PATCH v3 00/11] Introduce Simple atomic counters
Date: Fri, 9 Oct 2020 21:37:46 +0200	[thread overview]
Message-ID: <20201009193746.GA1073957@hirez.programming.kicks-ass.net> (raw)
In-Reply-To: <cover.1602209970.git.skhan@linuxfoundation.org>

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.

  parent reply	other threads:[~2020-10-09 19:38 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-09 15:55 [PATCH v3 00/11] Introduce Simple atomic counters Shuah Khan
2020-10-09 15:56 ` [PATCH v3 05/11] drivers/acpi: convert seqno counter_atomic32 Shuah Khan
2020-10-09 15:56 ` [PATCH v3 06/11] drivers/acpi/apei: " Shuah Khan
2020-10-09 18:03 ` [PATCH v3 00/11] Introduce Simple atomic counters Kees Cook
2020-10-09 19:02   ` Shuah Khan
2020-10-09 19:37 ` Peter Zijlstra [this message]
2020-10-09 20:45   ` Kees Cook
2020-10-10 11:09     ` Peter Zijlstra
2020-10-14  2:12       ` Shuah Khan
2020-10-14  9:17         ` Peter Zijlstra
2020-10-14 23:31           ` Kees Cook
2020-10-16 10:53             ` Peter Zijlstra
2020-10-16 22:51               ` Kees Cook
2020-11-10 18:49                 ` Dan Carpenter
2020-10-16 21:56             ` Shuah Khan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201009193746.GA1073957@hirez.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=arnd@arndb.de \
    --cc=arve@android.com \
    --cc=bp@alien8.de \
    --cc=christian@brauner.io \
    --cc=corbet@lwn.net \
    --cc=devel@driverdev.osuosl.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=hridya@google.com \
    --cc=james.morse@arm.com \
    --cc=joel@joelfernandes.org \
    --cc=johannes@sipsolutions.net \
    --cc=keescook@chromium.org \
    --cc=lenb@kernel.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-edac@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=maco@android.com \
    --cc=mchehab@kernel.org \
    --cc=minyard@acm.org \
    --cc=openipmi-developer@lists.sourceforge.net \
    --cc=rafael@kernel.org \
    --cc=rric@kernel.org \
    --cc=shuah@kernel.org \
    --cc=skhan@linuxfoundation.org \
    --cc=surenb@google.com \
    --cc=tkjos@android.com \
    --cc=tony.luck@intel.com \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).