lkml.org 
[lkml]   [2020]   [Sep]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC PATCH 07/11] drivers/android/binder: convert stats, transaction_log to counter_atomic
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

\
 
 \ /
  Last update: 2020-09-23 22:51    [W:0.236 / U:0.204 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site