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