Messages in this thread Patch in this message | | | Date | Wed, 30 Oct 2019 11:39:24 -0700 | From | Kees Cook <> | Subject | Re: [PATCH v4 06/10] lib/refcount: Move saturation warnings out of line |
| |
On Wed, Oct 30, 2019 at 02:30:31PM +0000, Will Deacon wrote: > Having the refcount saturation and warnings inline bloats the text, > despite the fact that these paths should never be executed in normal > operation. > > Move the refcount saturation and warnings out of line to reduce the > image size when refcount_t checking is enabled. Relative to an x86_64 > defconfig, the sizes reported by bloat-o-meter are: > > # defconfig+REFCOUNT_FULL, inline saturation (i.e. before this patch) > Total: Before=14762076, After=14915442, chg +1.04% > > # defconfig+REFCOUNT_FULL, out-of-line saturation (i.e. after this patch) > Total: Before=14762076, After=14835497, chg +0.50% > > A side-effect of this change is that we now only get one warning per > refcount saturation type, rather than one per problematic call-site. > > Cc: Ingo Molnar <mingo@kernel.org> > Cc: Elena Reshetova <elena.reshetova@intel.com> > Cc: Peter Zijlstra <peterz@infradead.org> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> > Cc: Kees Cook <keescook@chromium.org> > Signed-off-by: Will Deacon <will@kernel.org>
Reviewed-by: Kees Cook <keescook@chromium.org>
-Kees
> --- > include/linux/refcount.h | 39 ++++++++++++++++++++------------------- > lib/refcount.c | 28 ++++++++++++++++++++++++++++ > 2 files changed, 48 insertions(+), 19 deletions(-) > > diff --git a/include/linux/refcount.h b/include/linux/refcount.h > index e3b218d669ce..1cd0a876a789 100644 > --- a/include/linux/refcount.h > +++ b/include/linux/refcount.h > @@ -23,6 +23,16 @@ typedef struct refcount_struct { > > #define REFCOUNT_INIT(n) { .refs = ATOMIC_INIT(n), } > > +enum refcount_saturation_type { > + REFCOUNT_ADD_NOT_ZERO_OVF, > + REFCOUNT_ADD_OVF, > + REFCOUNT_ADD_UAF, > + REFCOUNT_SUB_UAF, > + REFCOUNT_DEC_LEAK, > +}; > + > +void refcount_warn_saturate(refcount_t *r, enum refcount_saturation_type t); > + > /** > * refcount_set - set a refcount's value > * @r: the refcount > @@ -154,10 +164,8 @@ static inline __must_check bool refcount_add_not_zero(int i, refcount_t *r) > break; > } while (!atomic_try_cmpxchg_relaxed(&r->refs, &old, old + i)); > > - if (unlikely(old < 0 || old + i < 0)) { > - refcount_set(r, REFCOUNT_SATURATED); > - WARN_ONCE(1, "refcount_t: saturated; leaking memory.\n"); > - } > + if (unlikely(old < 0 || old + i < 0)) > + refcount_warn_saturate(r, REFCOUNT_ADD_NOT_ZERO_OVF); > > return old; > } > @@ -182,11 +190,10 @@ static inline void refcount_add(int i, refcount_t *r) > { > int old = atomic_fetch_add_relaxed(i, &r->refs); > > - WARN_ONCE(!old, "refcount_t: addition on 0; use-after-free.\n"); > - if (unlikely(old <= 0 || old + i <= 0)) { > - refcount_set(r, REFCOUNT_SATURATED); > - WARN_ONCE(old, "refcount_t: saturated; leaking memory.\n"); > - } > + if (unlikely(!old)) > + refcount_warn_saturate(r, REFCOUNT_ADD_UAF); > + else if (unlikely(old < 0 || old + i < 0)) > + refcount_warn_saturate(r, REFCOUNT_ADD_OVF); > } > > /** > @@ -253,10 +260,8 @@ static inline __must_check bool refcount_sub_and_test(int i, refcount_t *r) > return true; > } > > - if (unlikely(old < 0 || old - i < 0)) { > - refcount_set(r, REFCOUNT_SATURATED); > - WARN_ONCE(1, "refcount_t: underflow; use-after-free.\n"); > - } > + if (unlikely(old < 0 || old - i < 0)) > + refcount_warn_saturate(r, REFCOUNT_SUB_UAF); > > return false; > } > @@ -291,12 +296,8 @@ static inline __must_check bool refcount_dec_and_test(refcount_t *r) > */ > static inline void refcount_dec(refcount_t *r) > { > - int old = atomic_fetch_sub_release(1, &r->refs); > - > - if (unlikely(old <= 1)) { > - refcount_set(r, REFCOUNT_SATURATED); > - WARN_ONCE(1, "refcount_t: decrement hit 0; leaking memory.\n"); > - } > + if (unlikely(atomic_fetch_sub_release(1, &r->refs) <= 1)) > + refcount_warn_saturate(r, REFCOUNT_DEC_LEAK); > } > #else /* CONFIG_REFCOUNT_FULL */ > > diff --git a/lib/refcount.c b/lib/refcount.c > index 3a534fbebdcc..8b7e249c0e10 100644 > --- a/lib/refcount.c > +++ b/lib/refcount.c > @@ -8,6 +8,34 @@ > #include <linux/spinlock.h> > #include <linux/bug.h> > > +#define REFCOUNT_WARN(str) WARN_ONCE(1, "refcount_t: " str ".\n") > + > +void refcount_warn_saturate(refcount_t *r, enum refcount_saturation_type t) > +{ > + refcount_set(r, REFCOUNT_SATURATED); > + > + switch (t) { > + case REFCOUNT_ADD_NOT_ZERO_OVF: > + REFCOUNT_WARN("saturated; leaking memory"); > + break; > + case REFCOUNT_ADD_OVF: > + REFCOUNT_WARN("saturated; leaking memory"); > + break; > + case REFCOUNT_ADD_UAF: > + REFCOUNT_WARN("addition on 0; use-after-free"); > + break; > + case REFCOUNT_SUB_UAF: > + REFCOUNT_WARN("underflow; use-after-free"); > + break; > + case REFCOUNT_DEC_LEAK: > + REFCOUNT_WARN("decrement hit 0; leaking memory"); > + break; > + default: > + REFCOUNT_WARN("unknown saturation event!?"); > + } > +} > +EXPORT_SYMBOL(refcount_warn_saturate); > + > /** > * refcount_dec_if_one - decrement a refcount if it is 1 > * @r: the refcount > -- > 2.24.0.rc0.303.g954a862665-goog >
-- Kees Cook
| |