All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg KH <gregkh@linuxfoundation.org>
To: "Reshetova, Elena" <elena.reshetova@intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
	Liljestrand Hans <ishkamiel@gmail.com>,
	"kernel-hardening@lists.openwall.com"
	<kernel-hardening@lists.openwall.com>,
	Kees Cook <keescook@chromium.org>,
	"will.deacon@arm.com" <will.deacon@arm.com>,
	Boqun Feng <boqun.feng@gmail.com>,
	David Windsor <dwindsor@gmail.com>,
	"aik@ozlabs.ru" <aik@ozlabs.ru>,
	"david@gibson.dropbear.id.au" <david@gibson.dropbear.id.au>
Subject: [kernel-hardening] Re: Conversion from atomic_t to refcount_t: summary of issues
Date: Tue, 20 Dec 2016 10:30:51 +0100	[thread overview]
Message-ID: <20161220093051.GB12200@kroah.com> (raw)
In-Reply-To: <2236FBA76BA1254E88B949DDB74E612B41C22713@IRSMSX102.ger.corp.intel.com>

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. 

kernel programming is hard :)

Don't get frustrated, it's going to be a lot of work, just break it up
into chunks and go at it...

> 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)

Why not merge the "correct" implementation?  Don't submit something that
doesn't work well.  Then fix up the instances that are broken when you
convert them to this new api.

> 2) patch set that fixes all problematic places (potentially with code rewrite) 
> 3) patch that removes possibility of inc on zero from refcount_t

That implies that 3) would not happen for another year or so, not good.
Do it right the first time.

thanks,

greg k-h

  reply	other threads:[~2016-12-20  9:30 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-28 11:56 [kernel-hardening] Conversion from atomic_t to refcount_t: summary of issues Reshetova, Elena
2016-11-28 12:13 ` [kernel-hardening] " Peter Zijlstra
2016-11-28 12:44   ` Peter Zijlstra
2016-11-28 12:48   ` Peter Zijlstra
2016-11-28 14:12   ` [kernel-hardening] " Reshetova, Elena
2016-11-29  3:19   ` [kernel-hardening] " Alexey Kardashevskiy
2016-11-29  9:31     ` Peter Zijlstra
2016-11-30  0:23       ` Alexey Kardashevskiy
2016-11-29 15:35   ` [kernel-hardening] " Reshetova, Elena
2016-11-29 15:47     ` Peter Zijlstra
2016-12-01 19:15     ` [kernel-hardening] " Peter Zijlstra
2016-12-01 21:31       ` David Windsor
2016-12-01 23:03         ` Peter Zijlstra
2016-12-01 23:20           ` Kees Cook
2016-12-01 23:29             ` David Windsor
2016-12-02  1:17             ` Boqun Feng
2016-12-02 20:25               ` David Windsor
2016-12-07 13:24                 ` Peter Zijlstra
2016-12-07 19:03                   ` David Windsor
2016-12-09 14:48                     ` David Windsor
2016-12-07 13:36             ` Peter Zijlstra
2016-12-01 23:20           ` David Windsor
2016-12-07 13:21             ` Peter Zijlstra
2016-12-02 15:44       ` Liljestrand Hans
2016-12-02 16:14         ` Greg KH
2016-12-07 13:52         ` Peter Zijlstra
2016-12-07 15:59           ` David Windsor
2016-12-07 16:26             ` Peter Zijlstra
2016-12-07 16:31               ` David Windsor
2016-12-16 12:10           ` [kernel-hardening] " Reshetova, Elena
2016-12-16 14:01             ` [kernel-hardening] " Peter Zijlstra
2016-12-19  7:55               ` [kernel-hardening] " Reshetova, Elena
2016-12-19 10:12                 ` [kernel-hardening] " Peter Zijlstra
2016-12-20  9:13                   ` [kernel-hardening] " Reshetova, Elena
2016-12-20  9:30                     ` Greg KH [this message]
2016-12-20  9:40                       ` Reshetova, Elena
2016-12-20  9:51                         ` [kernel-hardening] " Greg KH
2016-12-20  9:55                           ` [kernel-hardening] " Reshetova, Elena
2016-12-20 10:26                             ` [kernel-hardening] " Greg KH
2016-12-20  9:41                     ` Peter Zijlstra
2016-12-20  9:58                       ` [kernel-hardening] " Reshetova, Elena
2016-12-20 10:55                       ` [kernel-hardening] " Liljestrand Hans
2016-12-20 13:13                         ` Peter Zijlstra
2016-12-20 13:35                           ` Reshetova, Elena
2016-12-20 15:20                           ` Liljestrand Hans
2016-12-20 15:52                             ` Peter Zijlstra
2017-01-10 14:58                             ` Peter Zijlstra
2016-12-07 14:13     ` Peter Zijlstra

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20161220093051.GB12200@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=aik@ozlabs.ru \
    --cc=boqun.feng@gmail.com \
    --cc=david@gibson.dropbear.id.au \
    --cc=dwindsor@gmail.com \
    --cc=elena.reshetova@intel.com \
    --cc=ishkamiel@gmail.com \
    --cc=keescook@chromium.org \
    --cc=kernel-hardening@lists.openwall.com \
    --cc=peterz@infradead.org \
    --cc=will.deacon@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.