linux-kernel - Re: [PATCH 09/11] drivers/char/ipmi: convert stats to use 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]

Date:   Fri, 25 Sep 2020 20:05:39 -0600
From:   Shuah Khan <skhan@...uxfoundation.org>
To:     minyard@....org
Cc:     arnd@...db.de, gregkh@...uxfoundation.org, keescook@...omium.org,
        openipmi-developer@...ts.sourceforge.net,
        linux-kernel@...r.kernel.org,
        Shuah Khan <skhan@...uxfoundation.org>
Subject: Re: [PATCH 09/11] drivers/char/ipmi: convert stats to use
 counter_atomic32

On 9/25/20 6:15 PM, Corey Minyard wrote:
> On Fri, Sep 25, 2020 at 05:47:23PM -0600, Shuah Khan wrote:
>> counter_atomic* is introduced to be used when a variable is used as
>> a simple counter and doesn't guard object lifetimes. This clearly
>> differentiates atomic_t usages that guard object lifetimes.
>>
>> counter_atomic* variables will wrap around to 0 when it overflows and
>> should not be used to guard resource lifetimes, device usage and
>> open counts that control state changes, and pm states.
>>
>> atomic_t variables used for stats are atomic counters. Overflow will
>> wrap around and reset the stats and no change with the conversion.
>>
>> Convert them to use counter_atomic32.
>>
>> Signed-off-by: Shuah Khan <skhan@...uxfoundation.org>
> 
> Reviewed-by: Corey Minyard <cminyard@...sta.com>
> 

Thanks Corey.

> I assume for this conversion that the plan is to eliminate atomic_t
> completely and convert all atomic counters used for object lifetime to
> struct kref?  The new naming is certainly more clear and I'm happy with
> this change.
> 

No plans to replace or get rid of atomic_t/refcount_t ops. The reason is
to clearly differentiate atomic_t uses that don't guard object lifetimes
or state management, hence prone to overflow and underflow errors.

By clearly differentiating the ones guard the lifetimes and that don't
using this new counter interface, the existing tools that scan for
overflow/underflow conditions can filter out Counters API and look for
the variables that truly guard the lifetimes. Currently it is becoming
very hard to zero in on the errors with the noise.

Second reason is, atomic_t is overused. Non-atomic counters in this
API can be used for stats/counters that don't need atomocity.

Hope this helps.

thanks,
-- Shuah