From: Kees Cook <keescook@chromium.org>
To: Greg KH <gregkh@linuxfoundation.org>
Cc: devel@driverdev.osuosl.org, tkjos@android.com, surenb@google.com,
linux-kernel@vger.kernel.org, hridya@google.com,
arve@android.com, Shuah Khan <skhan@linuxfoundation.org>,
joel@joelfernandes.org, maco@android.com, christian@brauner.io
Subject: Re: [RFC PATCH 07/11] drivers/android/binder: convert stats, transaction_log to counter_atomic
Date: Wed, 23 Sep 2020 13:51:29 -0700 [thread overview]
Message-ID: <202009231350.8343298C23@keescook> (raw)
In-Reply-To: <20200923193134.GD199068@kroah.com>
On Wed, Sep 23, 2020 at 09:31:34PM +0200, Greg KH wrote:
> On Wed, Sep 23, 2020 at 12:04:58PM -0700, Kees Cook wrote:
> > On Wed, Sep 23, 2020 at 07:10:27AM +0200, Greg KH wrote:
> > > On Tue, Sep 22, 2020 at 07:43:36PM -0600, Shuah Khan wrote:
> > > > struct binder_stats {
> > > > - atomic_t br[_IOC_NR(BR_FAILED_REPLY) + 1];
> > > > - atomic_t bc[_IOC_NR(BC_REPLY_SG) + 1];
> > > > - atomic_t obj_created[BINDER_STAT_COUNT];
> > > > - atomic_t obj_deleted[BINDER_STAT_COUNT];
> > > > + struct counter_atomic br[_IOC_NR(BR_FAILED_REPLY) + 1];
> > > > + struct counter_atomic bc[_IOC_NR(BC_REPLY_SG) + 1];
> > > > + struct counter_atomic obj_created[BINDER_STAT_COUNT];
> > > > + struct counter_atomic obj_deleted[BINDER_STAT_COUNT];
> > >
> > > These are just debugging statistics, no reason they have to be atomic
> > > variables at all and they should be able to just be "struct counter"
> > > variables instead.
> >
> > But there's no reason for them _not_ to be atomic. Please let's keep
> > this API as always safe. Why even provide a new foot-gun here?
>
> These are debugging things, how can you shoot yourself in the foot with
> that???
Because suddenly you might be trying to use these values for debugging
only to dig and dig to discover that because they were non-atomic, some
parallel race cause a counter to get dropped, etc.
Since we can design this API robustly, let's take the opportunity to do
so.
--
Kees Cook
next prev parent reply other threads:[~2020-09-23 20:51 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-23 1:43 [RFC PATCH 00/11] Introduce Simple atomic and non-atomic counters Shuah Khan
2020-09-23 1:43 ` [RFC PATCH 01/11] counters: Introduce counter and counter_atomic Shuah Khan
2020-09-23 10:35 ` Greg KH
2020-09-23 19:04 ` Kees Cook
2020-09-23 19:34 ` Greg KH
2020-09-23 20:54 ` Kees Cook
2020-09-23 20:48 ` Shuah Khan
2020-09-23 20:58 ` Kees Cook
2020-09-23 21:19 ` Shuah Khan
2020-09-23 22:04 ` Kees Cook
2020-09-23 1:43 ` [RFC PATCH 02/11] selftests:lib: add new test for counters Shuah Khan
2020-09-23 1:43 ` [RFC PATCH 03/11] drivers/base: convert deferred_trigger_count and probe_count to counter_atomic Shuah Khan
2020-09-23 10:30 ` Greg KH
2020-09-23 1:43 ` [RFC PATCH 04/11] drivers/base/devcoredump: convert devcd_count " Shuah Khan
2020-09-23 10:31 ` Greg KH
2020-09-23 1:43 ` [RFC PATCH 05/11] drivers/acpi: convert seqno counter_atomic Shuah Khan
2020-09-24 11:13 ` Rafael J. Wysocki
2020-09-24 15:08 ` Shuah Khan
2020-09-24 15:32 ` Rafael J. Wysocki
2020-09-23 1:43 ` [RFC PATCH 06/11] drivers/acpi/apei: " Shuah Khan
2020-09-23 1:43 ` [RFC PATCH 07/11] drivers/android/binder: convert stats, transaction_log to counter_atomic Shuah Khan
2020-09-23 5:10 ` Greg KH
2020-09-23 19:04 ` Kees Cook
2020-09-23 19:31 ` Greg KH
2020-09-23 20:51 ` Kees Cook [this message]
2020-09-23 1:43 ` [RFC PATCH 08/11] drivers/base/test/test_async_driver_probe: convert to use counter_atomic Shuah Khan
2020-09-23 10:33 ` Greg KH
2020-09-23 1:43 ` [RFC PATCH 09/11] drivers/char/ipmi: convert stats " Shuah Khan
2020-09-23 1:43 ` [RFC PATCH 10/11] drivers/misc/vmw_vmci: convert num guest devices counter to counter_atomic Shuah Khan
2020-09-23 10:29 ` Greg KH
2020-09-23 1:43 ` [RFC PATCH 11/11] drivers/edac: convert pci counters " 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=202009231350.8343298C23@keescook \
--to=keescook@chromium.org \
--cc=arve@android.com \
--cc=christian@brauner.io \
--cc=devel@driverdev.osuosl.org \
--cc=gregkh@linuxfoundation.org \
--cc=hridya@google.com \
--cc=joel@joelfernandes.org \
--cc=linux-kernel@vger.kernel.org \
--cc=maco@android.com \
--cc=skhan@linuxfoundation.org \
--cc=surenb@google.com \
--cc=tkjos@android.com \
/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