All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Reshetova, Elena" <elena.reshetova@intel.com>
To: Peter Zijlstra <peterz@infradead.org>, David Windsor <dave@progbits.org>
Cc: Kees Cook <keescook@chromium.org>,
	Greg KH <gregkh@linuxfoundation.org>,
	Will Deacon <will.deacon@arm.com>, Arnd Bergmann <arnd@arndb.de>,
	"Thomas Gleixner" <tglx@linutronix.de>,
	Ingo Molnar <mingo@kernel.org>, "H. Peter Anvin" <hpa@zytor.com>,
	LKML <linux-kernel@vger.kernel.org>,
	"Alexei Starovoitov" <alexei.starovoitov@gmail.com>,
	Daniel Borkmann <daniel@iogearbox.net>
Subject: RE: [RFC][PATCH 2/7] kref: Add kref_read()
Date: Thu, 17 Nov 2016 13:01:49 +0000	[thread overview]
Message-ID: <2236FBA76BA1254E88B949DDB74E612B41C14089@IRSMSX102.ger.corp.intel.com> (raw)
In-Reply-To: <20161117124339.GC3117@twins.programming.kicks-ass.net>


On Thu, Nov 17, 2016 at 07:30:29AM -0500, David Windsor wrote:
> On Thu, Nov 17, 2016 at 3:34 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> > No, its not a statistic. Also, I'm far from convinced stats_t is an 
> > actually useful thing to have.
> >
> 
> Regarding this, has there been any thought given as to how stats_t 
> will meaningfully differ from atomic_t?  If refcount_t is semantically 
> "atomic_t with reference counter overflow protection," what 
> services/guarantees does stats_t provide?  I cannot think of any that 
> don't require implementing overflow detection of some sort, which 
> incurs a performance hit.

>Afaict the whole point of stats_t was to allow overflow, since its only stats, nobody cares etc..

>I think the sole motivator is a general distaste of atomic_t, which isn't a good reason at all.

I don't think anyone has this as motivation. But atomic_t is so powerful and flexible that easily ends up being misused (as past CVEs shown). 
Even if we now find all occurrences of atomic_t used as refcounter (which we cannot actually guarantee in any case unless someone manually reads every line)
and convert it to refcount_t, we still have atomic_t type present and new usage of it as refount will crawl in. It is just a matter of time IMO. 

So, this approach still doesn't solve the main problem: abuse of atomic_t a refcounter and  security vulnerabilities as result.  
What other mechanisms can we think we can utilize to prevent it? 
- Checkpatch? Would be hard to write enough rules to find all possible patterns how creative people might use atomic as refcounter. 
- People reviewing the code? Many kernel vulnerabilities live outside of core kernel, where maintainers are careful about what gets in and what's not. Further you go from core kernel (especially when you reach non-upstream drivers), code review quality is less, possibility of mistake is higher, and on average this code has more vulnerabilities. We can't say "this is not upstream code, who cares", because we want Linux kernel to follow "secure by default" principle: to provide enough mechanisms in kernel itself to minimize risk of mistakes and vulnerabilities. I think atomic is a great example of such case. We need to make it hard for people to make mistakes with overflows when overflows actually matter. 
This was really a reason for our initial approach that provided "security by default". Certainly it had some issues (we all agree on this), but let's think how else can we provide "secure by default" protection for this.  

 

  reply	other threads:[~2016-11-17 14:30 UTC|newest]

Thread overview: 110+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-14 17:39 [RFC][PATCH 0/7] kref improvements Peter Zijlstra
2016-11-14 17:39 ` [RFC][PATCH 1/7] kref: Add KREF_INIT() Peter Zijlstra
2016-11-14 17:39 ` [RFC][PATCH 2/7] kref: Add kref_read() Peter Zijlstra
2016-11-14 18:16   ` Christoph Hellwig
2016-11-15  7:28     ` Greg KH
2016-11-15  7:47       ` Peter Zijlstra
2016-11-15  8:37       ` [PATCH] printk, locking/atomics, kref: Introduce new %pAr and %pAk format string options for atomic_t and 'struct kref' Ingo Molnar
2016-11-15  8:43         ` [PATCH v2] " Ingo Molnar
2016-11-15  9:21           ` Peter Zijlstra
2016-11-15  9:41             ` [PATCH v3] printk, locking/atomics, kref: Introduce new %pAa " Ingo Molnar
2016-11-15 10:10           ` [PATCH v2] printk, locking/atomics, kref: Introduce new %pAr " kbuild test robot
2016-11-15 16:42         ` [PATCH] " Linus Torvalds
2016-11-16  8:13           ` Ingo Molnar
2016-11-15  7:33   ` [RFC][PATCH 2/7] kref: Add kref_read() Greg KH
2016-11-15  8:03     ` Peter Zijlstra
2016-11-15 20:53       ` Kees Cook
2016-11-16  8:21         ` Greg KH
2016-11-16 10:10           ` Peter Zijlstra
2016-11-16 10:18             ` Greg KH
2016-11-16 10:11           ` Daniel Borkmann
2016-11-16 10:19             ` Greg KH
2016-11-16 10:09         ` Peter Zijlstra
2016-11-16 18:58           ` Kees Cook
2016-11-17  8:34             ` Peter Zijlstra
2016-11-17 12:30               ` David Windsor
2016-11-17 12:43                 ` Peter Zijlstra
2016-11-17 13:01                   ` Reshetova, Elena [this message]
2016-11-17 13:22                     ` Peter Zijlstra
2016-11-17 15:42                       ` Reshetova, Elena
2016-11-17 18:02                       ` Reshetova, Elena
2016-11-17 19:10                         ` Peter Zijlstra
2016-11-17 19:29                         ` Peter Zijlstra
2016-11-17 19:34               ` Kees Cook
2016-11-14 17:39 ` [RFC][PATCH 3/7] kref: Kill kref_sub() Peter Zijlstra
2016-11-14 17:39 ` [RFC][PATCH 4/7] kref: Use kref_get_unless_zero() more Peter Zijlstra
2016-11-14 17:39 ` [RFC][PATCH 5/7] kref: Implement kref_put_lock() Peter Zijlstra
2016-11-14 20:35   ` Kees Cook
2016-11-15  7:50     ` Peter Zijlstra
2016-11-14 17:39 ` [RFC][PATCH 6/7] kref: Avoid more abuse Peter Zijlstra
2016-11-14 17:39 ` [RFC][PATCH 7/7] kref: Implement using refcount_t Peter Zijlstra
2016-11-15  8:40   ` Ingo Molnar
2016-11-15  9:47     ` Peter Zijlstra
2016-11-15 10:03       ` Ingo Molnar
2016-11-15 10:46         ` Peter Zijlstra
2016-11-15 13:03           ` Ingo Molnar
2016-11-15 18:06             ` Kees Cook
2016-11-15 19:16               ` Peter Zijlstra
2016-11-15 19:23                 ` Kees Cook
2016-11-16  8:31                   ` Ingo Molnar
2016-11-16  8:51                     ` Greg KH
2016-11-16  9:07                       ` Ingo Molnar
2016-11-16  9:24                         ` Greg KH
2016-11-16 10:15                     ` Peter Zijlstra
2016-11-16 18:55                       ` Kees Cook
2016-11-17  8:33                         ` Peter Zijlstra
2016-11-17 19:50                           ` Kees Cook
2016-11-16 18:41                     ` Kees Cook
2016-11-15 12:33   ` Boqun Feng
2016-11-15 13:01     ` Peter Zijlstra
2016-11-15 14:19       ` Boqun Feng
2016-11-17  9:28         ` Peter Zijlstra
2016-11-17  9:48           ` Boqun Feng
2016-11-17 10:29             ` Peter Zijlstra
2016-11-17 10:39               ` Peter Zijlstra
2016-11-17 11:03                 ` Greg KH
2016-11-17 12:48                   ` Peter Zijlstra
     [not found]               ` <CAL0jBu-GnREUPSX4kUDp-Cc8ZGp6+Cb2q0HVandswcLzPRnChQ@mail.gmail.com>
2016-11-17 12:08                 ` Peter Zijlstra
2016-11-17 12:08           ` Will Deacon
2016-11-17 16:11             ` Peter Zijlstra
2016-11-17 16:36               ` Will Deacon
2016-11-18  8:26                 ` Boqun Feng
2016-11-18 10:16                   ` Will Deacon
2016-11-18 10:07   ` Reshetova, Elena
2016-11-18 11:37     ` Peter Zijlstra
2016-11-18 17:06       ` Will Deacon
2016-11-18 18:57         ` Peter Zijlstra
2016-11-21  4:06         ` Boqun Feng
2016-11-21  7:48           ` Ingo Molnar
2016-11-21  8:38             ` Boqun Feng
2016-11-21  8:44       ` Boqun Feng
2016-11-21  9:02         ` Peter Zijlstra
2016-11-21  9:37           ` Boqun Feng
2016-11-18 10:47   ` Reshetova, Elena
2016-11-18 10:52     ` Peter Zijlstra
2016-11-18 16:58       ` Reshetova, Elena
2016-11-18 18:53         ` Peter Zijlstra
2016-11-19  7:14           ` Reshetova, Elena
2016-11-19 11:45             ` Peter Zijlstra
2017-01-26 23:14   ` Kees Cook
2017-01-27  9:58     ` Peter Zijlstra
2017-01-27 21:07       ` Kees Cook
2017-01-30 13:40         ` Peter Zijlstra
2016-11-15  7:27 ` [RFC][PATCH 0/7] kref improvements Greg KH
2016-11-15  7:42   ` Ingo Molnar
2016-11-15 15:05     ` Greg KH
2016-11-15  7:48   ` Peter Zijlstra
2016-11-16 20:08 [RFC][PATCH 2/7] kref: Add kref_read() Alexei Starovoitov
2016-11-17  8:53 ` Peter Zijlstra
2016-11-17 16:19   ` Alexei Starovoitov
2016-11-17 16:34     ` Thomas Gleixner
2016-11-18 17:33     ` Reshetova, Elena
2016-11-19  3:47       ` Alexei Starovoitov
2016-11-21  8:18         ` Reshetova, Elena
2016-11-21 12:47       ` David Windsor
2016-11-21 15:39         ` Reshetova, Elena
2016-11-21 15:49           ` Peter Zijlstra
2016-11-21 16:00             ` Peter Zijlstra
2016-11-21 19:27               ` Reshetova, Elena
2016-11-21 20:12                 ` David Windsor
2016-11-22 10:37                   ` 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=2236FBA76BA1254E88B949DDB74E612B41C14089@IRSMSX102.ger.corp.intel.com \
    --to=elena.reshetova@intel.com \
    --cc=alexei.starovoitov@gmail.com \
    --cc=arnd@arndb.de \
    --cc=daniel@iogearbox.net \
    --cc=dave@progbits.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=hpa@zytor.com \
    --cc=keescook@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --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.