[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Fri, 2 Dec 2016 09:17:59 +0800
From: Boqun Feng <boqun.feng@...il.com>
To: Kees Cook <keescook@...omium.org>
Cc: Peter Zijlstra <peterz@...radead.org>,
David Windsor <dwindsor@...il.com>,
"Reshetova, Elena" <elena.reshetova@...el.com>,
"kernel-hardening@...ts.openwall.com" <kernel-hardening@...ts.openwall.com>,
Greg KH <gregkh@...uxfoundation.org>,
"will.deacon@....com" <will.deacon@....com>,
Hans Liljestrand <ishkamiel@...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 Thu, Dec 01, 2016 at 03:20:29PM -0800, Kees Cook wrote:
> On Thu, Dec 1, 2016 at 3:03 PM, Peter Zijlstra <peterz@...radead.org> wrote:
> > On Thu, Dec 01, 2016 at 04:31:16PM -0500, David Windsor wrote:
> >> Also, I'd like to point out that while identifying stats_t instances, I
> >> have found a similar distribution of non-standard functions (as agreed upon
> >> for the stats_t API).
> >
> >> First, usage of atomic_long_wrap_t (there currently isn't a stats_long_t
> >> planned for implementation):
> >
> > There isn't even a stats_t planned. I'm still very much not convinced
> > stats_t is needed or even makes sense.
> >
> > It wouldn't have any different semantics from atomic_t, and the only
> > argument Kees made was that reduced atomic_t usage would make it easier
> > to spot refcounts, but you're already building tools to find those.
> >
> > Once the tools work, who cares.
>
> The tool is only part of the whole thing. By distinctly splitting the
> other major atomic_t usage pattern away from atomic_t, it solidifies a
> stats_t as NOT a reference counter. It's the slow feature-creep or bad
> example situations that I'd like to avoid. Also, tools won't catch
> everything, and doing manual inspection is much easier if we know a
> stats_t cannot be misused.
>
> There doesn't seem to be a good reason NOT to have stats_t, beyond the
> work needed to create it and audit the places it should be used.
>
So we currently don't have a clear semantics for stats_t, do we? What
kind of atomic_t should be replaced with stats_t? In the link David
pointed out, there are a few cases where a stats_t is put on a
correctness-related variable. I don't think that's a good place to use
stats_t.
Besides, there are many statistics-purpose variables in kernel which are
not atomic, given those, only calling atomic statistic variables stats_t
seems inappropriate and misleading.
How about provide a modifier, say __stats, like __rcu, __percpu. We can
add it to the types of the variables that are only for
statistics-purpose. I think it won't be difficult to find all related
callsites of a __stats with the help of some compiler frontend tools, we
can then detect a possible problem if we do a cmpxchg on a __stats,
which is unlikely a proper usage for a statistic variable. And we don't
need to change or use special APIs, we just need to mark variables and
fields.
Thoughts?
Regards,
Boqun
> -Kees
>
> --
> Kees Cook
> Nexus Security
Download attachment "signature.asc" of type "application/pgp-signature" (456 bytes)
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.
