Date: Fri, 11 Nov 2016 09:43:00 -0800 From: Kees Cook <keescook@...omium.org> To: Peter Zijlstra <peterz@...radead.org>, Will Deacon <will.deacon@....com>, Greg KH <gregkh@...uxfoundation.org> Cc: David Windsor <dave@...gbits.org>, "kernel-hardening@...ts.openwall.com" <kernel-hardening@...ts.openwall.com>, Elena Reshetova <elena.reshetova@...el.com>, Arnd Bergmann <arnd@...db.de>, Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>, "H. Peter Anvin" <h.peter.anvin@...el.com> Subject: Re: Re: [RFC v4 PATCH 00/13] HARDENED_ATOMIC (And now Greg went missing from the reply? Re-added...) On Thu, Nov 10, 2016 at 11:50 PM, David Windsor <dave@...gbits.org> wrote: > On Thu, Nov 10, 2016 at 6:38 PM, Greg KH <gregkh@...uxfoundation.org> wrote: >> On Thu, Nov 10, 2016 at 03:15:44PM -0800, Kees Cook wrote: > While I really, really don't want to go down the opt-in route, here > are the three relevant types and their associated use cases: > > 1) kref: Used for honest-to-goodness reference counters that want > overflow protection. Uses a new type: atomic_nowrap_t that has > HARDENED_ATOMIC protection. Based on other feedback, it sounds like we're better off with refcount_t (which kref could be implemented on top of). And refcount_t would have a limited API: inc, dec_and_test (or whatever is determined as sanely minimal). > 2) statistical counters: Atomic in all cases, but wraps. Yup. sequence_t seems to make the most sense on naming, I think. If we want to get crazy, the type could be sequence_wrap_t. > 3) atomic_t: All other users of atomics (locks, etc.). Wrapping > behavior depends on a CONFIG setting. Correct: if CONFIG_PARANOID_ATOMIC (or something) is set, atomic_t is implemented as refcount_t, otherwise as sequence_t. As far as refcount_t is concerned, I worry using cmpxchg will be too costly, but it's worth benchmarking. I suspect the first pass at this style will be: 1) create refcount_t, API, and wrap detection 2) convert atomic_t-as-refcount users to refcount_t (coccinelle to find "dec_and_test" followed by "free"?) 3) create sequence_t API 4) convert atomic_t-as-stat users to sequence_t 5) implement atomic_t as one-or-the-other based on CONFIG 1,2 and 3,4 can happen at the same time. > We'd need to audit the kernel and identify all reference counter users > of atomic_t and convert them to kref. This isn't nearly as large a > task as enumerating all statistical counter users of atomic_t. > > The third category, users of atomic_t that fit neither into the > category of reference counters or stat counters, is the interesting > case. I'm drawing a blank as to which of these users would require > overflow protection? If none do, we could drastically reduce the size > of this patchset: leave atomic_t alone and implement kref with > atomic_nowrap_t. Again, it's very late and I'm drawing a blank as to > "misc" users of atomic_t (non-refcount and non-stat-counters). > >>> statistic counters (say, "statcount" implemented with new atomic_wrap_t) >> >> Lots of these are also "sequences", that drivers rely on. Hopefully >> they can wrap as that's what happens today. So that might not be the >> best name, but naming is hard... >> >>> everything else (named "atomic_t", implemented as either >>> atomic_nowrap_t or atomic_wrap_t, depending on CONFIG) >> > > I don't like rerfcount/refcount_t: kref should be used. In the same > spirit, kstatcount might be more consistent. > > Pure, unadulterated bikeshedding. =) I suspect the mechanical changes to using krefcount_t will be easier that switching to kref. Large comments around krefcount_t can be added to encourage people to just use kref, though. Peter, Will, Greg, how does this 5-step approach (above) sound to you? It gets us explicit marking in the kernel for what is expected to be a reference counter or a sequence counter, and give us the ability to go paranoid with the remaining atomic_t users. -Kees -- Kees Cook Nexus Security
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.