From: Peter Zijlstra <peterz@infradead.org>
To: "Reshetova, Elena" <elena.reshetova@intel.com>
Cc: "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
"keescook@chromium.org" <keescook@chromium.org>,
"will.deacon@arm.com" <will.deacon@arm.com>,
"arnd@arndb.de" <arnd@arndb.de>,
"tglx@linutronix.de" <tglx@linutronix.de>,
"mingo@kernel.org" <mingo@kernel.org>,
"hpa@zytor.com" <hpa@zytor.com>,
"dave@progbits.org" <dave@progbits.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
boqun.feng@gmail.com
Subject: Re: [RFC][PATCH 7/7] kref: Implement using refcount_t
Date: Fri, 18 Nov 2016 12:37:18 +0100 [thread overview]
Message-ID: <20161118113718.GP3117@twins.programming.kicks-ass.net> (raw)
In-Reply-To: <2236FBA76BA1254E88B949DDB74E612B41C148C4@IRSMSX102.ger.corp.intel.com>
On Fri, Nov 18, 2016 at 10:07:26AM +0000, Reshetova, Elena wrote:
>
> Peter do you have the changes to the refcount_t interface compare to
> the version in this patch?
> We are now starting working on atomic_t --> refcount_t conversions and
> it would save a bit of work to have latest version from you that we
> can be based upon.
The latestest version below, mostly just comment changes since last
time.
---
Subject: refcount_t: A special purpose refcount type
From: Peter Zijlstra <peterz@infradead.org>
Date: Mon Nov 14 18:06:19 CET 2016
Provide refcount_t, an atomic_t like primitive built just for
refcounting.
It provides saturation semantics such that overflow becomes impossible
and thereby 'spurious' use-after-free is avoided.
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
include/linux/refcount.h | 241 +++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 241 insertions(+)
--- /dev/null
+++ b/include/linux/refcount.h
@@ -0,0 +1,241 @@
+#ifndef _LINUX_REFCOUNT_H
+#define _LINUX_REFCOUNT_H
+
+/*
+ * Variant of atomic_t specialized for reference counts.
+ *
+ * The interface matches the atomic_t interface (to aid in porting) but only
+ * provides the few functions one should use for reference counting.
+ *
+ * It differs in that the counter saturates at UINT_MAX and will not move once
+ * there. This avoids wrapping the counter and causing 'spurious'
+ * use-after-free issues.
+ *
+ * Memory ordering rules are slightly relaxed wrt regular atomic_t functions
+ * and provide only what is strictly required for refcounts.
+ *
+ * The increments are fully relaxed; these will not provide ordering. The
+ * rationale is that whatever is used to obtain the object we're increasing the
+ * reference count on will provide the ordering. For locked data structures,
+ * its the lock acquire, for RCU/lockless data structures its the dependent
+ * load.
+ *
+ * Do note that inc_not_zero() provides a control dependency which will order
+ * future stores against the inc, this ensures we'll never modify the object
+ * if we did not in fact acquire a reference.
+ *
+ * The decrements will provide release order, such that all the prior loads and
+ * stores will be issued before, it also provides a control dependency, which
+ * will order us against the subsequent free().
+ *
+ * The control dependency is against the load of the cmpxchg (ll/sc) that
+ * succeeded. This means the stores aren't fully ordered, but this is fine
+ * because the 1->0 transition indicates no concurrency.
+ *
+ * Note that the allocator is responsible for ordering things between free()
+ * and alloc().
+ *
+ *
+ * Note: the implementation hard relies on increments, bigger than 1 additions
+ * need explicit overflow -> saturation logic.
+ *
+ */
+
+#include <linux/atomic.h>
+#include <linux/bug.h>
+#include <linux/mutex.h>
+#include <linux/spinlock.h>
+
+typedef struct refcount_struct {
+ atomic_t refs;
+} refcount_t;
+
+#define REFCOUNT_INIT(n) { .refs = ATOMIC_INIT(n), }
+
+static inline void refcount_set(refcount_t *r, int n)
+{
+ atomic_set(&r->refs, n);
+}
+
+static inline unsigned int refcount_read(const refcount_t *r)
+{
+ return atomic_read(&r->refs);
+}
+
+/*
+ * Similar to atomic_inc(), will saturate at UINT_MAX and WARN.
+ *
+ * Provides no memory ordering, it is assumed the caller already has a
+ * reference on the object, will WARN when this is not so.
+ */
+static inline void refcount_inc(refcount_t *r)
+{
+ unsigned int old, new, val = atomic_read(&r->refs);
+
+ for (;;) {
+ WARN(!val, "refcount_t: increment on 0; use-after-free.\n");
+
+ if (unlikely(val == UINT_MAX))
+ return;
+
+ new = val + 1;
+ old = atomic_cmpxchg_relaxed(&r->refs, val, new);
+ if (old == val)
+ break;
+
+ val = old;
+ }
+
+ WARN(new == UINT_MAX, "refcount_t: saturated; leaking memory.\n");
+}
+
+/*
+ * Similar to atomic_inc_not_zero(), will saturate at UINT_MAX and WARN.
+ *
+ * Provides no memory ordering, it is assumed the caller has guaranteed the
+ * object memory to be stable (RCU, etc.). It does provide a control dependency
+ * and thereby orders future stores. See the comment on top.
+ */
+static inline __must_check
+bool refcount_inc_not_zero(refcount_t *r)
+{
+ unsigned int old, new, val = atomic_read(&r->refs);
+
+ for (;;) {
+ if (!val)
+ return false;
+
+ if (unlikely(val == UINT_MAX))
+ return true;
+
+ new = val + 1;
+ old = atomic_cmpxchg_relaxed(&r->refs, val, new);
+ if (old == val)
+ break;
+
+ val = old;
+ }
+
+ WARN(new == UINT_MAX, "refcount_t: saturated; leaking memory.\n");
+
+ return true;
+}
+
+/*
+ * Similar to atomic_dec_and_test(), it will WARN on underflow and fail to
+ * decrement when saturated at UINT_MAX.
+ *
+ * Provides release memory ordering, such that prior loads and stores are done
+ * before, and provides a control dependency such that free() must come after.
+ * See the comment on top.
+ */
+static inline __must_check
+bool refcount_dec_and_test(refcount_t *r)
+{
+ unsigned int old, new, val = atomic_read(&r->refs);
+
+ for (;;) {
+ if (val == UINT_MAX)
+ return false;
+
+ new = val - 1;
+ if (WARN(new > val, "refcount_t: underflow; use-after-free.\n"))
+ return false;
+
+ old = atomic_cmpxchg_release(&r->refs, val, new);
+ if (old == val)
+ break;
+
+ val = old;
+ }
+
+ return !new;
+}
+
+/*
+ * Similar to atomic_dec_and_mutex_lock(), it will WARN on underflow and fail
+ * to decrement when saturated at UINT_MAX.
+ *
+ * Provides release memory ordering, such that prior loads and stores are done
+ * before, and provides a control dependency such that free() must come after.
+ * See the comment on top.
+ */
+static inline __must_check
+bool refcount_dec_and_mutex_lock(refcount_t *r, struct mutex *lock)
+{
+ unsigned int old, new, val = atomic_read(&r->refs);
+ bool locked = false;
+
+ for (;;) {
+ if (val == UINT_MAX)
+ return false;
+
+ if (val == 1 && !locked) {
+ locked = true;
+ mutex_lock(lock);
+ }
+
+ new = val - 1;
+ if (WARN(new > val, "refcount_t: underflow; use-after-free.\n")) {
+ if (locked)
+ mutex_unlock(lock);
+ return false;
+ }
+
+ old = atomic_cmpxchg_release(&r->refs, val, new);
+ if (old == val)
+ break;
+
+ val = old;
+ }
+
+ if (new && locked)
+ mutex_unlock(lock);
+
+ return !new;
+}
+
+/*
+ * Similar to atomic_dec_and_lock(), it will WARN on underflow and fail to
+ * decrement when saturated at UINT_MAX.
+ *
+ * Provides release memory ordering, such that prior loads and stores are done
+ * before, and provides a control dependency such that free() must come after.
+ * See the comment on top.
+ */
+static inline __must_check
+bool refcount_dec_and_lock(refcount_t *r, spinlock_t *lock)
+{
+ unsigned int old, new, val = atomic_read(&r->refs);
+ bool locked = false;
+
+ for (;;) {
+ if (val == UINT_MAX)
+ return false;
+
+ if (val == 1 && !locked) {
+ locked = true;
+ spin_lock(lock);
+ }
+
+ new = val - 1;
+ if (WARN(new > val, "refcount_t: underflow; use-after-free.\n")) {
+ if (locked)
+ spin_unlock(lock);
+ return false;
+ }
+
+ old = atomic_cmpxchg_release(&r->refs, val, new);
+ if (old == val)
+ break;
+
+ val = old;
+ }
+
+ if (new && locked)
+ spin_unlock(lock);
+
+ return !new;
+}
+
+#endif /* _LINUX_REFCOUNT_H */
next prev parent reply other threads:[~2016-11-18 11:37 UTC|newest]
Thread overview: 96+ 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
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 [this message]
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
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=20161118113718.GP3117@twins.programming.kicks-ass.net \
--to=peterz@infradead.org \
--cc=arnd@arndb.de \
--cc=boqun.feng@gmail.com \
--cc=dave@progbits.org \
--cc=elena.reshetova@intel.com \
--cc=gregkh@linuxfoundation.org \
--cc=hpa@zytor.com \
--cc=keescook@chromium.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.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.