[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2236FBA76BA1254E88B949DDB74E612B41C22797@IRSMSX102.ger.corp.intel.com>
Date: Tue, 20 Dec 2016 09:58:01 +0000
From: "Reshetova, Elena" <elena.reshetova@...el.com>
To: Peter Zijlstra <peterz@...radead.org>
CC: Liljestrand Hans <ishkamiel@...il.com>,
"kernel-hardening@...ts.openwall.com" <kernel-hardening@...ts.openwall.com>,
Greg KH <gregkh@...uxfoundation.org>, Kees Cook <keescook@...omium.org>,
"will.deacon@....com" <will.deacon@....com>, Boqun Feng
<boqun.feng@...il.com>, David Windsor <dwindsor@...il.com>, "aik@...abs.ru"
<aik@...abs.ru>, "david@...son.dropbear.id.au" <david@...son.dropbear.id.au>
Subject: RE: Conversion from atomic_t to refcount_t: summary of issues
> On Tue, Dec 20, 2016 at 09:13:58AM +0000, Reshetova, Elena wrote:
> > > On Mon, Dec 19, 2016 at 07:55:15AM +0000, Reshetova, Elena wrote:
> > > > Well, again, you are right in theory, but in practice for example for struct
> > > sched_group { atomic_t ref; ... }:
> > > >
> > > > http://lxr.free-electrons.com/source/kernel/sched/core.c#L6178
> > > >
> > > > To me this is a refcounter that needs the protection.
> > >
> > > Only if you have more than UINT_MAX CPUs or something like that.
> > >
> > > And if you really really want to use refcount_t there, you could +1 the
> > > scheme and it'd work again.
> >
> > Well, yes, probably, but there are many cases like this in practice,
> > so we would need to have a good plan how to get it all submitted and
> > tested properly. The current patch set is already bigger than what we
> > had before and it is only growing. Hans will provide more info later
> > today based on his testing, which shows many places in kernel core
> > where we DO actually have increment on zero happening in practice and
> > whole kernel doesn't even boot with the strictest approach (refusing
> > to inc on zero). And we are only able to test for x86....
> >
> > Given the massive amount of changes, it would be good to merge this at
> > least in couple of stages:
> >
> > 1) first soft version of refcount_t API which at least allows
> > increment on zero and all atomic_t used as refcounter occurrences that
> > don't require reference counter scheme change (+1 or other) 2) patch
> > set that fixes all problematic places (potentially with code rewrite)
> > 3) patch that removes possibility of inc on zero from refcount_t
>
> I don't get it. Why ?
>
> Just leave the weird and problematic cases using atomic_t. Its far
> harder to remove crap later.
If it is internal change (not API), should be quite easy to remove later. But ok, I guess for really weird cases, it might be easier to postpone their change until the next patch set. And if we get help in testing, then hopefully it will go without major breakdowns.
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
