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
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
