Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1521929 > unrolled thread
| Started by | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| First post | 2016-11-14 18:50 +0100 |
| Last post | 2016-11-17 20:40 +0100 |
| Articles | 9 on this page of 89 — 12 participants |
Back to article view | Back to linux.kernel
[RFC][PATCH 0/7] kref improvements Peter Zijlstra <peterz@infradead.org> - 2016-11-14 18:50 +0100
[RFC][PATCH 5/7] kref: Implement kref_put_lock() Peter Zijlstra <peterz@infradead.org> - 2016-11-14 18:50 +0100
Re: [RFC][PATCH 5/7] kref: Implement kref_put_lock() Kees Cook <keescook@chromium.org> - 2016-11-14 21:40 +0100
Re: [RFC][PATCH 5/7] kref: Implement kref_put_lock() Peter Zijlstra <peterz@infradead.org> - 2016-11-15 09:00 +0100
[RFC][PATCH 6/7] kref: Avoid more abuse Peter Zijlstra <peterz@infradead.org> - 2016-11-14 18:50 +0100
[RFC][PATCH 4/7] kref: Use kref_get_unless_zero() more Peter Zijlstra <peterz@infradead.org> - 2016-11-14 18:50 +0100
[RFC][PATCH 7/7] kref: Implement using refcount_t Peter Zijlstra <peterz@infradead.org> - 2016-11-14 18:50 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Ingo Molnar <mingo@kernel.org> - 2016-11-15 09:50 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Peter Zijlstra <peterz@infradead.org> - 2016-11-15 10:50 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Ingo Molnar <mingo@kernel.org> - 2016-11-15 11:10 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Peter Zijlstra <peterz@infradead.org> - 2016-11-15 11:50 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Ingo Molnar <mingo@kernel.org> - 2016-11-15 14:10 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Kees Cook <keescook@chromium.org> - 2016-11-15 19:10 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Peter Zijlstra <peterz@infradead.org> - 2016-11-15 20:20 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Kees Cook <keescook@chromium.org> - 2016-11-15 20:30 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Ingo Molnar <mingo@kernel.org> - 2016-11-16 09:40 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Greg KH <gregkh@linuxfoundation.org> - 2016-11-16 10:00 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Ingo Molnar <mingo@kernel.org> - 2016-11-16 10:10 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Greg KH <gregkh@linuxfoundation.org> - 2016-11-16 10:30 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Peter Zijlstra <peterz@infradead.org> - 2016-11-16 11:20 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Kees Cook <keescook@chromium.org> - 2016-11-16 20:00 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Peter Zijlstra <peterz@infradead.org> - 2016-11-17 09:40 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Kees Cook <keescook@chromium.org> - 2016-11-17 21:00 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Kees Cook <keescook@chromium.org> - 2016-11-16 19:50 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Boqun Feng <boqun.feng@gmail.com> - 2016-11-15 13:40 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Peter Zijlstra <peterz@infradead.org> - 2016-11-15 14:10 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Boqun Feng <boqun.feng@gmail.com> - 2016-11-15 15:20 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Peter Zijlstra <peterz@infradead.org> - 2016-11-17 10:30 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Boqun Feng <boqun.feng@gmail.com> - 2016-11-17 10:50 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Peter Zijlstra <peterz@infradead.org> - 2016-11-17 11:40 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Peter Zijlstra <peterz@infradead.org> - 2016-11-17 11:50 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Greg KH <gregkh@linuxfoundation.org> - 2016-11-17 12:10 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Peter Zijlstra <peterz@infradead.org> - 2016-11-17 18:10 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Peter Zijlstra <peterz@infradead.org> - 2016-11-17 18:30 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Will Deacon <will.deacon@arm.com> - 2016-11-17 18:10 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Boqun Feng <boqun.feng@gmail.com> - 2016-11-18 09:30 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Will Deacon <will.deacon@arm.com> - 2016-11-18 11:20 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Will Deacon <will.deacon@arm.com> - 2016-11-17 18:10 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Peter Zijlstra <peterz@infradead.org> - 2016-11-17 18:30 +0100
RE: [RFC][PATCH 7/7] kref: Implement using refcount_t "Reshetova, Elena" <elena.reshetova@intel.com> - 2016-11-18 11:10 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Peter Zijlstra <peterz@infradead.org> - 2016-11-18 12:40 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Will Deacon <will.deacon@arm.com> - 2016-11-18 18:10 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Peter Zijlstra <peterz@infradead.org> - 2016-11-18 20:00 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Boqun Feng <boqun.feng@gmail.com> - 2016-11-21 05:10 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Ingo Molnar <mingo@kernel.org> - 2016-11-21 08:50 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Boqun Feng <boqun.feng@gmail.com> - 2016-11-21 09:40 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Boqun Feng <boqun.feng@gmail.com> - 2016-11-21 09:50 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Peter Zijlstra <peterz@infradead.org> - 2016-11-21 10:10 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Boqun Feng <boqun.feng@gmail.com> - 2016-11-21 10:40 +0100
RE: [RFC][PATCH 7/7] kref: Implement using refcount_t "Reshetova, Elena" <elena.reshetova@intel.com> - 2016-11-18 11:50 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Peter Zijlstra <peterz@infradead.org> - 2016-11-18 12:00 +0100
RE: [RFC][PATCH 7/7] kref: Implement using refcount_t "Reshetova, Elena" <elena.reshetova@intel.com> - 2016-11-18 18:00 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Peter Zijlstra <peterz@infradead.org> - 2016-11-18 20:00 +0100
RE: [RFC][PATCH 7/7] kref: Implement using refcount_t "Reshetova, Elena" <elena.reshetova@intel.com> - 2016-11-19 08:20 +0100
Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Peter Zijlstra <peterz@infradead.org> - 2016-11-19 12:50 +0100
Re: [RFC][PATCH 2/7] kref: Add kref_read() Christoph Hellwig <hch@infradead.org> - 2016-11-14 19:20 +0100
Re: [RFC][PATCH 2/7] kref: Add kref_read() Greg KH <gregkh@linuxfoundation.org> - 2016-11-15 08:30 +0100
Re: [RFC][PATCH 2/7] kref: Add kref_read() Peter Zijlstra <peterz@infradead.org> - 2016-11-15 08:50 +0100
[PATCH] printk, locking/atomics, kref: Introduce new %pAr and %pAk format string options for atomic_t and 'struct kref' Ingo Molnar <mingo@kernel.org> - 2016-11-15 09:40 +0100
[PATCH v2] printk, locking/atomics, kref: Introduce new %pAr and %pAk format string options for atomic_t and 'struct kref' Ingo Molnar <mingo@kernel.org> - 2016-11-15 09:50 +0100
Re: [PATCH v2] printk, locking/atomics, kref: Introduce new %pAr and %pAk format string options for atomic_t and 'struct kref' Peter Zijlstra <peterz@infradead.org> - 2016-11-15 10:30 +0100
[PATCH v3] printk, locking/atomics, kref: Introduce new %pAa and %pAk format string options for atomic_t and 'struct kref' Ingo Molnar <mingo@kernel.org> - 2016-11-15 10:50 +0100
Re: [PATCH v2] printk, locking/atomics, kref: Introduce new %pAr and %pAk format string options for atomic_t and 'struct kref' kbuild test robot <lkp@intel.com> - 2016-11-15 11:10 +0100
Re: [PATCH] printk, locking/atomics, kref: Introduce new %pAr and %pAk format string options for atomic_t and 'struct kref' Linus Torvalds <torvalds@linux-foundation.org> - 2016-11-15 17:50 +0100
Re: [PATCH] printk, locking/atomics, kref: Introduce new %pAr and %pAk format string options for atomic_t and 'struct kref' Ingo Molnar <mingo@kernel.org> - 2016-11-16 09:20 +0100
Re: [RFC][PATCH 0/7] kref improvements Greg KH <gregkh@linuxfoundation.org> - 2016-11-15 08:30 +0100
Re: [RFC][PATCH 0/7] kref improvements Ingo Molnar <mingo@kernel.org> - 2016-11-15 08:50 +0100
Re: [RFC][PATCH 0/7] kref improvements Greg KH <gregkh@linuxfoundation.org> - 2016-11-15 16:10 +0100
Re: [RFC][PATCH 0/7] kref improvements Peter Zijlstra <peterz@infradead.org> - 2016-11-15 08:50 +0100
Re: [RFC][PATCH 2/7] kref: Add kref_read() Greg KH <gregkh@linuxfoundation.org> - 2016-11-15 08:40 +0100
Re: [RFC][PATCH 2/7] kref: Add kref_read() Peter Zijlstra <peterz@infradead.org> - 2016-11-15 09:10 +0100
Re: [RFC][PATCH 2/7] kref: Add kref_read() Kees Cook <keescook@chromium.org> - 2016-11-15 22:00 +0100
Re: [RFC][PATCH 2/7] kref: Add kref_read() Greg KH <gregkh@linuxfoundation.org> - 2016-11-16 09:30 +0100
Re: [RFC][PATCH 2/7] kref: Add kref_read() Peter Zijlstra <peterz@infradead.org> - 2016-11-16 11:20 +0100
Re: [RFC][PATCH 2/7] kref: Add kref_read() Greg KH <gregkh@linuxfoundation.org> - 2016-11-16 11:20 +0100
Re: [RFC][PATCH 2/7] kref: Add kref_read() Greg KH <gregkh@linuxfoundation.org> - 2016-11-16 11:20 +0100
Re: [RFC][PATCH 2/7] kref: Add kref_read() Daniel Borkmann <daniel@iogearbox.net> - 2016-11-16 11:20 +0100
Re: [RFC][PATCH 2/7] kref: Add kref_read() Peter Zijlstra <peterz@infradead.org> - 2016-11-16 11:10 +0100
Re: [RFC][PATCH 2/7] kref: Add kref_read() Kees Cook <keescook@chromium.org> - 2016-11-16 20:00 +0100
Re: [RFC][PATCH 2/7] kref: Add kref_read() Peter Zijlstra <peterz@infradead.org> - 2016-11-17 09:40 +0100
Re: [RFC][PATCH 2/7] kref: Add kref_read() David Windsor <dave@progbits.org> - 2016-11-17 13:50 +0100
RE: [RFC][PATCH 2/7] kref: Add kref_read() "Reshetova, Elena" <elena.reshetova@intel.com> - 2016-11-17 15:40 +0100
Re: [RFC][PATCH 2/7] kref: Add kref_read() Peter Zijlstra <peterz@infradead.org> - 2016-11-17 18:10 +0100
RE: [RFC][PATCH 2/7] kref: Add kref_read() "Reshetova, Elena" <elena.reshetova@intel.com> - 2016-11-17 18:10 +0100
RE: [RFC][PATCH 2/7] kref: Add kref_read() "Reshetova, Elena" <elena.reshetova@intel.com> - 2016-11-17 19:10 +0100
Re: [RFC][PATCH 2/7] kref: Add kref_read() Peter Zijlstra <peterz@infradead.org> - 2016-11-17 20:20 +0100
Re: [RFC][PATCH 2/7] kref: Add kref_read() Peter Zijlstra <peterz@infradead.org> - 2016-11-17 20:40 +0100
Re: [RFC][PATCH 2/7] kref: Add kref_read() Peter Zijlstra <peterz@infradead.org> - 2016-11-17 18:20 +0100
Re: [RFC][PATCH 2/7] kref: Add kref_read() Kees Cook <keescook@chromium.org> - 2016-11-17 20:40 +0100
Page 5 of 5 — ← Prev page 1 2 3 4 [5]
| From | David Windsor <dave@progbits.org> |
|---|---|
| Date | 2016-11-17 13:50 +0100 |
| Subject | Re: [RFC][PATCH 2/7] kref: Add kref_read() |
| Message-ID | <sEuiB-JC-3@gated-at.bofh.it> |
| In reply to | #1524163 |
On Thu, Nov 17, 2016 at 3:34 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Wed, Nov 16, 2016 at 10:58:38AM -0800, Kees Cook wrote:
>> On Wed, Nov 16, 2016 at 2:09 AM, Peter Zijlstra <peterz@infradead.org> wrote:
>> > On Tue, Nov 15, 2016 at 12:53:35PM -0800, Kees Cook wrote:
>> >>
>> >> What should we do about things like this (bpf_prog_put() and callbacks
>> >> from kernel/bpf/syscall.c):
>> >>
>> >>
>> >> static void bpf_prog_uncharge_memlock(struct bpf_prog *prog)
>> >> {
>> >> struct user_struct *user = prog->aux->user;
>> >>
>> >> atomic_long_sub(prog->pages, &user->locked_vm);
>> >> free_uid(user);
>> >> }
>> >>
>> >> static void __bpf_prog_put_rcu(struct rcu_head *rcu)
>> >> {
>> >> struct bpf_prog_aux *aux = container_of(rcu, struct bpf_prog_aux, rcu);
>> >>
>> >> free_used_maps(aux);
>> >> bpf_prog_uncharge_memlock(aux->prog);
>> >> bpf_prog_free(aux->prog);
>> >> }
>> >>
>> >> void bpf_prog_put(struct bpf_prog *prog)
>> >> {
>> >> if (atomic_dec_and_test(&prog->aux->refcnt))
>> >> call_rcu(&prog->aux->rcu, __bpf_prog_put_rcu);
>> >> }
>> >>
>> >>
>> >> Not only do we want to protect prog->aux->refcnt, but I think we want
>> >> to protect user->locked_vm too ... I don't think it's sane for
>> >> user->locked_vm to be a stats_t ?
>> >
>> > Why would you want to mess with locked_vm? You seem of the opinion that
>> > everything atomic_t is broken, this isn't the case.
>>
>> What I mean to say is that while the refcnt here should clearly be
>> converted to kref or refcount_t, it looks like locked_vm should become
>> a new stats_t. However, it seems weird for locked_vm to ever wrap
>> either...
>
> 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.
One conceivable service/guarantee would be to give stats_t the ability
to detect/report when an overflow has occurred, but not ultimately
with the offending process getting killed. On x86, this could be
done by having stats_t overflows generate a different exception number
and corresponding handler than refcount_t-generated overflows. It
would still contain the mechanisms for detecting and responding to
overflows, but the response to stats_t overflows would differ from
that of refcount_t overflows. Semantically, this version of stats_t
would be "refcount_t minus 'kill the offending process'." I'm not
sure if this abstraction is in fact useful, or indeed worth the
requisite performance hit; I'm just suggesting a possible semantic
difference between atomic_t and stats_t.
> refcount_t brought special semantics that clearly are different from
> regular atomic_t, stats_t would not, so why would it need to exist.
>
> Not to mention that you seem over eager to apply it, which doesn't
> inspire confidence.
[toc] | [prev] | [next] | [standalone]
| From | "Reshetova, Elena" <elena.reshetova@intel.com> |
|---|---|
| Date | 2016-11-17 15:40 +0100 |
| Subject | RE: [RFC][PATCH 2/7] kref: Add kref_read() |
| Message-ID | <sEw15-1T0-73@gated-at.bofh.it> |
| In reply to | #1524443 |
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.
[toc] | [prev] | [next] | [standalone]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2016-11-17 18:10 +0100 |
| Subject | Re: [RFC][PATCH 2/7] kref: Add kref_read() |
| Message-ID | <sEymd-3v8-5@gated-at.bofh.it> |
| In reply to | #1524462 |
On Thu, Nov 17, 2016 at 01:01:49PM +0000, Reshetova, Elena wrote:
> >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).
I don't think using atomic_t as reference count is abuse. There simply
wasn't anything better. The proposed refcount_t cures this.
> 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.
Improve tooling. The patterns shouldn't be _that_ hard to find. Once the
tools are good, new code isn't a problem either.
Anything: atomic*_{{dec,sub}_and_test,{add,sub}_return,fetch_{add,sub}}
followed by a call_rcu()/free().
[toc] | [prev] | [next] | [standalone]
| From | "Reshetova, Elena" <elena.reshetova@intel.com> |
|---|---|
| Date | 2016-11-17 18:10 +0100 |
| Subject | RE: [RFC][PATCH 2/7] kref: Add kref_read() |
| Message-ID | <sEymf-3v8-103@gated-at.bofh.it> |
| In reply to | #1524479 |
> 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.
>Improve tooling. The patterns shouldn't be _that_ hard to find. Once the tools are good, new code isn't a problem either.
>Anything: atomic*_{{dec,sub}_and_test,{add,sub}_return,fetch_{add,sub}}
>followed by a call_rcu()/free().
Does not find everything unfortunately. Even if you add to above atomic*_add_unless() and also things like schedule_work(), still I fear we aren't covering everything.
What is worse, I don't think there is a mechanism to guarantee full coverage.
[toc] | [prev] | [next] | [standalone]
| From | "Reshetova, Elena" <elena.reshetova@intel.com> |
|---|---|
| Date | 2016-11-17 19:10 +0100 |
| Subject | RE: [RFC][PATCH 2/7] kref: Add kref_read() |
| Message-ID | <sEzii-48J-43@gated-at.bofh.it> |
| In reply to | #1524479 |
> 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. >Improve tooling. The patterns shouldn't be _that_ hard to find. Once the tools are good, new code isn't a problem either. Moreover, thinking of out of tree drivers: you think they would always do checkpatch or run some of our tools for security checks?
[toc] | [prev] | [next] | [standalone]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2016-11-17 20:20 +0100 |
| Subject | Re: [RFC][PATCH 2/7] kref: Add kref_read() |
| Message-ID | <sEAo2-4ON-27@gated-at.bofh.it> |
| In reply to | #1524640 |
On Thu, Nov 17, 2016 at 06:02:33PM +0000, Reshetova, Elena wrote: > > > 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. > > >Improve tooling. The patterns shouldn't be _that_ hard to find. Once the tools are good, new code isn't a problem either. > > Moreover, thinking of out of tree drivers: you think they would always > do checkpatch or run some of our tools for security checks? If they can't be arsed, neither can I. You can't fix the unfixable. Like I said before, its chasing unicorns.
[toc] | [prev] | [next] | [standalone]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2016-11-17 20:40 +0100 |
| Subject | Re: [RFC][PATCH 2/7] kref: Add kref_read() |
| Message-ID | <sEAHp-4VW-71@gated-at.bofh.it> |
| In reply to | #1524640 |
On Thu, Nov 17, 2016 at 06:02:33PM +0000, Reshetova, Elena wrote: > >Improve tooling. The patterns shouldn't be _that_ hard to find. Once the tools are good, new code isn't a problem either. > > Moreover, thinking of out of tree drivers: you think they would always > do checkpatch or run some of our tools for security checks? Also, checkpatch is a horrid example. That's mostly meaningless and menial noise. Nobody wants to run that, even if, between all the gibberish it lists a few sensible things. Make an always enabled GCC plugin that generates build warns with a low enough false positive rate and nobody will complain.
[toc] | [prev] | [next] | [standalone]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2016-11-17 18:20 +0100 |
| Subject | Re: [RFC][PATCH 2/7] kref: Add kref_read() |
| Message-ID | <sEw15-1T0-75@gated-at.bofh.it> |
| In reply to | #1524443 |
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.
[toc] | [prev] | [next] | [standalone]
| From | Kees Cook <keescook@chromium.org> |
|---|---|
| Date | 2016-11-17 20:40 +0100 |
| Subject | Re: [RFC][PATCH 2/7] kref: Add kref_read() |
| Message-ID | <sEAHp-4VW-73@gated-at.bofh.it> |
| In reply to | #1524163 |
On Thu, Nov 17, 2016 at 12:34 AM, Peter Zijlstra <peterz@infradead.org> wrote: > On Wed, Nov 16, 2016 at 10:58:38AM -0800, Kees Cook wrote: >> What I mean to say is that while the refcnt here should clearly be >> converted to kref or refcount_t, it looks like locked_vm should become >> a new stats_t. However, it seems weird for locked_vm to ever wrap >> either... > > No, its not a statistic. Also, I'm far from convinced stats_t is an > actually useful thing to have. It's useful because its introduction creates a type that can't be trivially used for refcounting (i.e. hard to make the mistake of using stats_t for refcounting), and replacing atomic_t statistic counters with stats_t reduces the effort required to do the initial (and on-going) audit for misuse of atomic_t as a refcounter. > refcount_t brought special semantics that clearly are different from > regular atomic_t, stats_t would not, so why would it need to exist. Your original suggestion about stats_t showed how its accessor API would be a very small subset of the regular atomic_t set. I think that reduction in accidental misuse has value. > Not to mention that you seem over eager to apply it, which doesn't > inspire confidence. I'd like to get to the point where auditing for mistakes in this area is tractable. :) If atomic_t is only used for non-stats and non-refcount, it's much much easier to examine and reason about. -Kees -- Kees Cook Nexus Security
[toc] | [prev] | [standalone]
Page 5 of 5 — ← Prev page 1 2 3 4 [5]
Back to top | Article view | linux.kernel
csiph-web