linux-kernel - Re: [RFC PATCH 01/11] counters: Introduce counter and counter

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <202009231502.20F24788E@keescook>
Date:   Wed, 23 Sep 2020 15:04:35 -0700
From:   Kees Cook <keescook@...omium.org>
To:     Shuah Khan <skhan@...uxfoundation.org>
Cc:     corbet@....net, gregkh@...uxfoundation.org,
        linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH 01/11] counters: Introduce counter and counter_atomic

On Wed, Sep 23, 2020 at 03:19:08PM -0600, Shuah Khan wrote:
> On 9/23/20 2:58 PM, Kees Cook wrote:
> > On Wed, Sep 23, 2020 at 02:48:22PM -0600, Shuah Khan wrote:
> > > On 9/23/20 1:04 PM, Kees Cook wrote:
> > > > On Tue, Sep 22, 2020 at 07:43:30PM -0600, Shuah Khan wrote:
> > > > I would really like these APIs to be _impossible_ to use for object
> > > > lifetime management. To that end, I would like to have all of the
> > > > *_return() functions removed. It should be strictly init, inc, dec,
> > > > read.
> > > > 
> > > 
> > > Yes. I am with you on making this API as small as possible so it won't
> > > be used for lifetime mgmt. That means no support for:
> > > 
> > > *_test, add_negative etc.
> > > 
> > > I started out with just init, inc, dec, read. As I started looking
> > > for candidates that can be converted to counters, I found inc_return()
> > > usages. I think we need inc_return() for sure. I haven't come across
> > > atomic_dec_return() yet.
> > 
> > What are the inc_return() cases? If they're not "safe" to use inc() and
> > then read(), then those likely need a closer look at what they're doing.
> > 
> 
> 3 in this series I sent. I would say I barely scratched the surface
> when it comes to finding candidates for converting.
> 
> drivers/android/binder.c
> drivers/acpi/acpi_extlog.c
> drivers/acpi/apei/ghes.c
> 
> These uses look reasonable to me. Having this inc_return() will save
> making _inc() followed by _read()

I'd like to make sure it's clear that it should not be treated as atomic
(even if it is), so a separate _read(), I think, makes that clear. And
hopefully it'll keep people from ever trying to sneak a _dec_return()
in. :)

> I will change counter to counter_simple and add a warning that this
> should only be used when atomic isn't needed. I can outline some
> tips for choosing the right one.

Okay.

-- 
Kees Cook