Skip to content

Commit dcb7864

Browse files
willdeaconIngo Molnar
authored andcommitted
locking/refcount: Improve performance of generic REFCOUNT_FULL code
Rewrite the generic REFCOUNT_FULL implementation so that the saturation point is moved to INT_MIN / 2. This allows us to defer the sanity checks until after the atomic operation, which removes many uses of cmpxchg() in favour of atomic_fetch_{add,sub}(). Some crude perf results obtained from lkdtm show substantially less overhead, despite the checking: $ perf stat -r 3 -B -- echo {ATOMIC,REFCOUNT}_TIMING >/sys/kernel/debug/provoke-crash/DIRECT # arm64 ATOMIC_TIMING: 46.50451 +- 0.00134 seconds time elapsed ( +- 0.00% ) REFCOUNT_TIMING (REFCOUNT_FULL, mainline): 77.57522 +- 0.00982 seconds time elapsed ( +- 0.01% ) REFCOUNT_TIMING (REFCOUNT_FULL, this series): 48.7181 +- 0.0256 seconds time elapsed ( +- 0.05% ) # x86 ATOMIC_TIMING: 31.6225 +- 0.0776 seconds time elapsed ( +- 0.25% ) REFCOUNT_TIMING (!REFCOUNT_FULL, mainline/x86 asm): 31.6689 +- 0.0901 seconds time elapsed ( +- 0.28% ) REFCOUNT_TIMING (REFCOUNT_FULL, mainline): 53.203 +- 0.138 seconds time elapsed ( +- 0.26% ) REFCOUNT_TIMING (REFCOUNT_FULL, this series): 31.7408 +- 0.0486 seconds time elapsed ( +- 0.15% ) Signed-off-by: Will Deacon <[email protected]> Reviewed-by: Ard Biesheuvel <[email protected]> Reviewed-by: Kees Cook <[email protected]> Tested-by: Hanjun Guo <[email protected]> Tested-by: Jan Glauber <[email protected]> Cc: Ard Biesheuvel <[email protected]> Cc: Elena Reshetova <[email protected]> Cc: Linus Torvalds <[email protected]> Cc: Peter Zijlstra <[email protected]> Cc: Thomas Gleixner <[email protected]> Link: https://lkml.kernel.org/r/[email protected] Signed-off-by: Ingo Molnar <[email protected]>
1 parent 77e9971 commit dcb7864

File tree

1 file changed

+75
-56
lines changed

1 file changed

+75
-56
lines changed

include/linux/refcount.h

Lines changed: 75 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -47,18 +47,56 @@ static inline unsigned int refcount_read(const refcount_t *r)
4747
#ifdef CONFIG_REFCOUNT_FULL
4848
#include <linux/bug.h>
4949

50-
#define REFCOUNT_MAX (UINT_MAX - 1)
51-
#define REFCOUNT_SATURATED UINT_MAX
50+
#define REFCOUNT_MAX INT_MAX
51+
#define REFCOUNT_SATURATED (INT_MIN / 2)
5252

5353
/*
5454
* Variant of atomic_t specialized for reference counts.
5555
*
5656
* The interface matches the atomic_t interface (to aid in porting) but only
5757
* provides the few functions one should use for reference counting.
5858
*
59-
* It differs in that the counter saturates at REFCOUNT_SATURATED and will not
60-
* move once there. This avoids wrapping the counter and causing 'spurious'
61-
* use-after-free issues.
59+
* Saturation semantics
60+
* ====================
61+
*
62+
* refcount_t differs from atomic_t in that the counter saturates at
63+
* REFCOUNT_SATURATED and will not move once there. This avoids wrapping the
64+
* counter and causing 'spurious' use-after-free issues. In order to avoid the
65+
* cost associated with introducing cmpxchg() loops into all of the saturating
66+
* operations, we temporarily allow the counter to take on an unchecked value
67+
* and then explicitly set it to REFCOUNT_SATURATED on detecting that underflow
68+
* or overflow has occurred. Although this is racy when multiple threads
69+
* access the refcount concurrently, by placing REFCOUNT_SATURATED roughly
70+
* equidistant from 0 and INT_MAX we minimise the scope for error:
71+
*
72+
* INT_MAX REFCOUNT_SATURATED UINT_MAX
73+
* 0 (0x7fff_ffff) (0xc000_0000) (0xffff_ffff)
74+
* +--------------------------------+----------------+----------------+
75+
* <---------- bad value! ---------->
76+
*
77+
* (in a signed view of the world, the "bad value" range corresponds to
78+
* a negative counter value).
79+
*
80+
* As an example, consider a refcount_inc() operation that causes the counter
81+
* to overflow:
82+
*
83+
* int old = atomic_fetch_add_relaxed(r);
84+
* // old is INT_MAX, refcount now INT_MIN (0x8000_0000)
85+
* if (old < 0)
86+
* atomic_set(r, REFCOUNT_SATURATED);
87+
*
88+
* If another thread also performs a refcount_inc() operation between the two
89+
* atomic operations, then the count will continue to edge closer to 0. If it
90+
* reaches a value of 1 before /any/ of the threads reset it to the saturated
91+
* value, then a concurrent refcount_dec_and_test() may erroneously free the
92+
* underlying object. Given the precise timing details involved with the
93+
* round-robin scheduling of each thread manipulating the refcount and the need
94+
* to hit the race multiple times in succession, there doesn't appear to be a
95+
* practical avenue of attack even if using refcount_add() operations with
96+
* larger increments.
97+
*
98+
* Memory ordering
99+
* ===============
62100
*
63101
* Memory ordering rules are slightly relaxed wrt regular atomic_t functions
64102
* and provide only what is strictly required for refcounts.
@@ -109,25 +147,19 @@ static inline unsigned int refcount_read(const refcount_t *r)
109147
*/
110148
static inline __must_check bool refcount_add_not_zero(int i, refcount_t *r)
111149
{
112-
unsigned int new, val = atomic_read(&r->refs);
150+
int old = refcount_read(r);
113151

114152
do {
115-
if (!val)
116-
return false;
117-
118-
if (unlikely(val == REFCOUNT_SATURATED))
119-
return true;
120-
121-
new = val + i;
122-
if (new < val)
123-
new = REFCOUNT_SATURATED;
153+
if (!old)
154+
break;
155+
} while (!atomic_try_cmpxchg_relaxed(&r->refs, &old, old + i));
124156

125-
} while (!atomic_try_cmpxchg_relaxed(&r->refs, &val, new));
126-
127-
WARN_ONCE(new == REFCOUNT_SATURATED,
128-
"refcount_t: saturated; leaking memory.\n");
157+
if (unlikely(old < 0 || old + i < 0)) {
158+
refcount_set(r, REFCOUNT_SATURATED);
159+
WARN_ONCE(1, "refcount_t: saturated; leaking memory.\n");
160+
}
129161

130-
return true;
162+
return old;
131163
}
132164

133165
/**
@@ -148,7 +180,13 @@ static inline __must_check bool refcount_add_not_zero(int i, refcount_t *r)
148180
*/
149181
static inline void refcount_add(int i, refcount_t *r)
150182
{
151-
WARN_ONCE(!refcount_add_not_zero(i, r), "refcount_t: addition on 0; use-after-free.\n");
183+
int old = atomic_fetch_add_relaxed(i, &r->refs);
184+
185+
WARN_ONCE(!old, "refcount_t: addition on 0; use-after-free.\n");
186+
if (unlikely(old <= 0 || old + i <= 0)) {
187+
refcount_set(r, REFCOUNT_SATURATED);
188+
WARN_ONCE(old, "refcount_t: saturated; leaking memory.\n");
189+
}
152190
}
153191

154192
/**
@@ -166,23 +204,7 @@ static inline void refcount_add(int i, refcount_t *r)
166204
*/
167205
static inline __must_check bool refcount_inc_not_zero(refcount_t *r)
168206
{
169-
unsigned int new, val = atomic_read(&r->refs);
170-
171-
do {
172-
new = val + 1;
173-
174-
if (!val)
175-
return false;
176-
177-
if (unlikely(!new))
178-
return true;
179-
180-
} while (!atomic_try_cmpxchg_relaxed(&r->refs, &val, new));
181-
182-
WARN_ONCE(new == REFCOUNT_SATURATED,
183-
"refcount_t: saturated; leaking memory.\n");
184-
185-
return true;
207+
return refcount_add_not_zero(1, r);
186208
}
187209

188210
/**
@@ -199,7 +221,7 @@ static inline __must_check bool refcount_inc_not_zero(refcount_t *r)
199221
*/
200222
static inline void refcount_inc(refcount_t *r)
201223
{
202-
WARN_ONCE(!refcount_inc_not_zero(r), "refcount_t: increment on 0; use-after-free.\n");
224+
refcount_add(1, r);
203225
}
204226

205227
/**
@@ -224,26 +246,19 @@ static inline void refcount_inc(refcount_t *r)
224246
*/
225247
static inline __must_check bool refcount_sub_and_test(int i, refcount_t *r)
226248
{
227-
unsigned int new, val = atomic_read(&r->refs);
228-
229-
do {
230-
if (unlikely(val == REFCOUNT_SATURATED))
231-
return false;
249+
int old = atomic_fetch_sub_release(i, &r->refs);
232250

233-
new = val - i;
234-
if (new > val) {
235-
WARN_ONCE(new > val, "refcount_t: underflow; use-after-free.\n");
236-
return false;
237-
}
238-
239-
} while (!atomic_try_cmpxchg_release(&r->refs, &val, new));
240-
241-
if (!new) {
251+
if (old == i) {
242252
smp_acquire__after_ctrl_dep();
243253
return true;
244254
}
245-
return false;
246255

256+
if (unlikely(old < 0 || old - i < 0)) {
257+
refcount_set(r, REFCOUNT_SATURATED);
258+
WARN_ONCE(1, "refcount_t: underflow; use-after-free.\n");
259+
}
260+
261+
return false;
247262
}
248263

249264
/**
@@ -276,9 +291,13 @@ static inline __must_check bool refcount_dec_and_test(refcount_t *r)
276291
*/
277292
static inline void refcount_dec(refcount_t *r)
278293
{
279-
WARN_ONCE(refcount_dec_and_test(r), "refcount_t: decrement hit 0; leaking memory.\n");
280-
}
294+
int old = atomic_fetch_sub_release(1, &r->refs);
281295

296+
if (unlikely(old <= 1)) {
297+
refcount_set(r, REFCOUNT_SATURATED);
298+
WARN_ONCE(1, "refcount_t: decrement hit 0; leaking memory.\n");
299+
}
300+
}
282301
#else /* CONFIG_REFCOUNT_FULL */
283302

284303
#define REFCOUNT_MAX INT_MAX

0 commit comments

Comments
 (0)