refcount: Create unchecked atomic_t implementation
PATCH v3 1/1 refcount: Create unchecked atomic_t implementation
Many subsystems will not use refcount_t unless there is a way to build the kernel so that there is no regression in speed compared to atomic_t. This adds CONFIG_REFCOUNT_FULL to enable the full refcount_t implementation which has the validation but is slightly slower. When not enabled, refcount_t uses the basic unchecked atomic_t routines, which results in no code changes compared to just using atomic_t directly.
---
v3: unbreak slightly long lines; Ingo. Add Greg's Ack.
v2: use better atomic ops; Elena and Peter.
--- arch/Kconfig | 9 +++++++++ include/linux/refcount.h | 42 ++++++++++++++++++++++++++++++++++++++++++ lib/refcount.c | 3 +++ 3 files changed, 54 insertions(+)
arch/Kconfig +9 -0
@@ -867,4 +867,13 @@ config STRICT_MODULE_RWX config ARCH_WANT_RELAX_ORDER bool +config REFCOUNT_FULL + bool "Perform full reference count validation at the expense of speed" + help + Enabling this switches the refcounting infrastructure from a fast + unchecked atomic_t implementation to a fully state checked + implementation, which can be slower but provides protections + against various use-after-free conditions that can be used in + security flaw exploits. + source "kernel/gcov/Kconfig"
include/linux/refcount.h +42 -0
54 lines not on this page · raw message
lib/refcount.c +3 -0
@@ -37,6 +37,8 @@#include <linux/refcount.h> #include <linux/bug.h> +#ifdef CONFIG_REFCOUNT_FULL +/** * refcount_add_not_zero - add a value to a refcount unless it is 0 * @i: the value to add to the refcount
@@ -225,6 +227,7 @@ void refcount_dec(refcount_t *r) WARN_ONCE(refcount_dec_and_test(r), "refcount_t: decrement hit 0; leaking memory.\n"); }EXPORT_SYMBOL (refcount_dec); +#endif /* CONFIG_REFCOUNT_FULL */ /** * refcount_dec_if_one - decrement a refcount if it is 1
Signature · 6 lines
2.7.4 -- Kees Cook Pixel Security
tip-bot for Kees Cook
merge bot[tip:locking/core] locking/refcount: Create unchecked atomic_t implementation
locking/refcount: Create unchecked atomic_t implementation
Commit-ID: 87f7583e9205fe5220677d0775f3a4d2b8fe8827 Gitweb: http://git.kernel.org/tip/87f7583e9205fe5220677d0775f3a4d2b8fe8827 Author: Kees Cook <[email protected]> AuthorDate: Wed, 21 Jun 2017 13:00:26 -0700 Committer: Ingo Molnar <[email protected]> CommitDate: Thu, 22 Jun 2017 10:36:25 +0200
locking/refcount: Create unchecked atomic_t implementation
Many subsystems will not use refcount_t unless there is a way to build the kernel so that there is no regression in speed compared to atomic_t. This adds CONFIG_REFCOUNT_FULL to enable the full refcount_t implementation which has the validation but is slightly slower. When not enabled, refcount_t uses the basic unchecked atomic_t routines, which results in no code changes compared to just using atomic_t directly.
--- arch/Kconfig | 9 +++++++++ include/linux/refcount.h | 42 ++++++++++++++++++++++++++++++++++++++++++ lib/refcount.c | 3 +++ 3 files changed, 54 insertions(+)
arch/Kconfig +9 -0
@@ -867,4 +867,13 @@ config STRICT_MODULE_RWX config ARCH_WANT_RELAX_ORDER bool +config REFCOUNT_FULL + bool "Perform full reference count validation at the expense of speed" + help + Enabling this switches the refcounting infrastructure from a fast + unchecked atomic_t implementation to a fully state checked + implementation, which can be (slightly) slower but provides protections + against various use-after-free conditions that can be used in + security flaw exploits. + source "kernel/gcov/Kconfig"
include/linux/refcount.h +42 -0
54 lines not on this page · raw message
lib/refcount.c +3 -0
@@ -37,6 +37,8 @@#include <linux/refcount.h> #include <linux/bug.h> +#ifdef CONFIG_REFCOUNT_FULL +/** * refcount_add_not_zero - add a value to a refcount unless it is 0 * @i: the value to add to the refcount
@@ -225,6 +227,7 @@ void refcount_dec(refcount_t *r) WARN_ONCE(refcount_dec_and_test(r), "refcount_t: decrement hit 0; leaking memory.\n"); }EXPORT_SYMBOL (refcount_dec); +#endif /* CONFIG_REFCOUNT_FULL */ /** * refcount_dec_if_one - decrement a refcount if it is 1
tip-bot for Kees Cook
merge bot[tip:locking/core] locking/refcount: Create unchecked atomic_t implementation
In reply to Kees Cooklocking/refcount: Create unchecked atomic_t implementation
Commit-ID: fd25d19f6b8da315332bb75936605fb45d3ea981 Gitweb: http://git.kernel.org/tip/fd25d19f6b8da315332bb75936605fb45d3ea981 Author: Kees Cook <[email protected]> AuthorDate: Wed, 21 Jun 2017 13:00:26 -0700 Committer: Ingo Molnar <[email protected]> CommitDate: Wed, 28 Jun 2017 18:54:46 +0200
locking/refcount: Create unchecked atomic_t implementation
Many subsystems will not use refcount_t unless there is a way to build the kernel so that there is no regression in speed compared to atomic_t. This adds CONFIG_REFCOUNT_FULL to enable the full refcount_t implementation which has the validation but is slightly slower. When not enabled, refcount_t uses the basic unchecked atomic_t routines, which results in no code changes compared to just using atomic_t directly.
--- arch/Kconfig | 9 +++++++++ include/linux/refcount.h | 42 ++++++++++++++++++++++++++++++++++++++++++ lib/refcount.c | 3 +++ 3 files changed, 54 insertions(+)
arch/Kconfig +9 -0
@@ -867,4 +867,13 @@ config STRICT_MODULE_RWX config ARCH_WANT_RELAX_ORDER bool +config REFCOUNT_FULL + bool "Perform full reference count validation at the expense of speed" + help + Enabling this switches the refcounting infrastructure from a fast + unchecked atomic_t implementation to a fully state checked + implementation, which can be (slightly) slower but provides protections + against various use-after-free conditions that can be used in + security flaw exploits. + source "kernel/gcov/Kconfig"
include/linux/refcount.h +42 -0
54 lines not on this page · raw message
lib/refcount.c +3 -0
@@ -37,6 +37,8 @@#include <linux/refcount.h> #include <linux/bug.h> +#ifdef CONFIG_REFCOUNT_FULL +/** * refcount_add_not_zero - add a value to a refcount unless it is 0 * @i: the value to add to the refcount
@@ -225,6 +227,7 @@ void refcount_dec(refcount_t *r) WARN_ONCE(refcount_dec_and_test(r), "refcount_t: decrement hit 0; leaking memory.\n"); }EXPORT_SYMBOL (refcount_dec); +#endif /* CONFIG_REFCOUNT_FULL */ /** * refcount_dec_if_one - decrement a refcount if it is 1
Peter Zijlstra
maintainerRe: [tip:locking/core] locking/refcount: Create unchecked atomic_t implementation
Quoted 1 line · tip-bot for Kees Cook
locking/refcount: Create unchecked
atomic_timplementation
This seems to do only half the job. Here's the rest.
---
Subject: locking/refcount: Finish unchecked atomic_t implementation
For some reason the unchecked atomic_t implementation stopped half-way through, complete it it.
--- include/linux/refcount.h | 35 ++++++++++++++++++++++++++++++----- lib/Makefile | 7 +++---- lib/refcount.c | 3 --- 3 files changed, 33 insertions(+), 12 deletions(-)
include/linux/refcount.h +30 -5
@@ -52,10 +52,17 @@ extern __must_check bool refcount_sub_and_test(unsigned int i, refcount_t *r); extern __must_check bool refcount_dec_and_test(refcount_t *r); extern void refcount_dec(refcount_t *r); + +extern __must_check bool refcount_dec_if_one(refcount_t *r); +extern __must_check bool refcount_dec_not_one(refcount_t *r); +extern __must_check bool refcount_dec_and_mutex_lock(refcount_t *r, struct mutex *lock); +extern __must_check bool refcount_dec_and_lock(refcount_t *r, spinlock_t *lock); + #else # ifdef CONFIG_ARCH_HAS_REFCOUNT # include <asm/refcount.h> # else + static inline __must_check bool refcount_add_not_zero(unsigned int i, refcount_t *r) { return atomic_add_unless(&r->refs, i, 0);
@@ -90,12 +97,30 @@ static inline void refcount_dec(refcount_t *r) {atomic_dec (&r->refs); } + +static inline __must_check bool refcount_dec_if_one (refcount_t *r) +{ +int val =1 ; + +return atomic_try_cmpxchg_release (&r->refs, &val,0 ); +} + +static inline __must_check bool refcount_dec_not_one (refcount_t *r) +{ +return atomic_add_unless (&r->refs,-1 ,1 ); +} + +static inline __must_check bool refcount_dec_and_mutex_lock (refcount_t *r,struct mutex *lock) +{ +return atomic_dec_and_mutex_lock (&r->refs, lock); +} + +static inline __must_check bool refcount_dec_and_lock (refcount_t *r,spinlock_t *lock) +{ +return atomic_dec_and_lock (&r->refs, lock); +} +# endif /* !CONFIG_ARCH_HAS_REFCOUNT */ #endif /* CONFIG_REFCOUNT_FULL */ -extern __must_check bool refcount_dec_if_one (refcount_t *r); -extern __must_check bool refcount_dec_not_one (refcount_t *r); -extern __must_check bool refcount_dec_and_mutex_lock (refcount_t *r,struct mutex *lock); -extern __must_check bool refcount_dec_and_lock (refcount_t *r,spinlock_t *lock); -#endif /* _LINUX_REFCOUNT_H */
lib/Makefile +3 -4
@@ -38,12 +38,11 @@ obj-y += bcd.o div64.o sort.o parser.o debug_locks.o random32.o \ gcd.o lcm.o list_sort.o uuid.o flex_array.o iov_iter.o clz_ctz.o \ bsearch.o find_bit.o llist.o memweight.o kfifo.o \ percpu-refcount.o percpu_ida.o rhashtable.o reciprocal_div.o \ - once.o refcount.o usercopy.o errseq.o -obj-y += string_helpers.o + once.o usercopy.o errseq.o string_helpers.o hexdump.o kstrtox.o + +obj-$(CONFIG_REFCOUNT_FULL) += refcount.o obj-$(CONFIG_TEST_STRING_HELPERS) += test-string_helpers.o -obj-y += hexdump.o obj-$(CONFIG_TEST_HEXDUMP) += test_hexdump.o -obj-y += kstrtox.o obj-$(CONFIG_TEST_BPF) += test_bpf.o obj-$(CONFIG_TEST_FIRMWARE) += test_firmware.o obj-$(CONFIG_TEST_SYSCTL) += test_sysctl.o
lib/refcount.c +0 -3
@@ -37,8 +37,6 @@#include <linux/refcount.h> #include <linux/bug.h> -#ifdef CONFIG_REFCOUNT_FULL -/** * refcount_add_not_zero - add a value to a refcount unless it is 0 * @i: the value to add to the refcount
@@ -227,7 +225,6 @@ void refcount_dec(refcount_t *r)WARN_ONCE (refcount_dec_and_test (r),"refcount_t: decrement hit 0; leaking memory.\n" ); }EXPORT_SYMBOL (refcount_dec); -#endif /* CONFIG_REFCOUNT_FULL */ /** * refcount_dec_if_one - decrement a refcount if it is 1
Re: [tip:locking/core] locking/refcount: Create unchecked atomic_t implementation
Quoted 10 lines · Peter Zijlstra
Quoted 1 line · tip-bot for Kees Cook
locking/refcount: Create unchecked
atomic_timplementationThis seems to do only half the job. Here's the rest.
---
Subject: locking/refcount: Finish uncheckedatomic_timplementationFor some reason the unchecked
atomic_timplementation stopped half-way through, complete it it.
Hmm? The reason is that the implementation of the remaining functions is unchanged between full, unchecked, and x86.
Quoted 108 lines
Fixes fd25d19f6b8d ("locking/refcount: Create unchecked atomic_t implementation")Signed-off-by Peter Zijlstra (Intel) <[email protected]>--- include/linux/refcount.h | 35 ++++++++++++++++++++++++++++++----- lib/Makefile | 7 +++---- lib/refcount.c | 3 --- 3 files changed, 33 insertions(+), 12 deletions(-)include/linux/refcount.h +30
-5@@ -52,10 +52,17 @@ extern __must_check bool refcount_sub_and_test(unsigned int i, refcount_t *r); extern __must_check bool refcount_dec_and_test(refcount_t *r); extern void refcount_dec(refcount_t *r); + +extern __must_check bool refcount_dec_if_one(refcount_t *r); +extern __must_check bool refcount_dec_not_one(refcount_t *r); +extern __must_check bool refcount_dec_and_mutex_lock(refcount_t *r, struct mutex *lock); +extern __must_check bool refcount_dec_and_lock(refcount_t *r, spinlock_t *lock); + #else # ifdef CONFIG_ARCH_HAS_REFCOUNT # include <asm/refcount.h> # else + static inline __must_check bool refcount_add_not_zero(unsigned int i, refcount_t *r) { return atomic_add_unless(&r->refs, i, 0);@@ -90,12 +97,30 @@ static inline void refcount_dec(refcount_t *r) {atomic_dec (&r->refs); } + +static inline __must_check bool refcount_dec_if_one (refcount_t *r) +{ +int val =1 ; + +return atomic_try_cmpxchg_release (&r->refs, &val,0 ); +} + +static inline __must_check bool refcount_dec_not_one (refcount_t *r) +{ +return atomic_add_unless (&r->refs,-1 ,1 ); +} + +static inline __must_check bool refcount_dec_and_mutex_lock (refcount_t *r,struct mutex *lock) +{ +return atomic_dec_and_mutex_lock (&r->refs, lock); +} + +static inline __must_check bool refcount_dec_and_lock (refcount_t *r,spinlock_t *lock) +{ +return atomic_dec_and_lock (&r->refs, lock); +} +# endif /* !CONFIG_ARCH_HAS_REFCOUNT */ #endif /* CONFIG_REFCOUNT_FULL */ -extern __must_check bool refcount_dec_if_one (refcount_t *r); -extern __must_check bool refcount_dec_not_one (refcount_t *r); -extern __must_check bool refcount_dec_and_mutex_lock (refcount_t *r,struct mutex *lock); -extern __must_check bool refcount_dec_and_lock (refcount_t *r,spinlock_t *lock); -#endif /* _LINUX_REFCOUNT_H */ lib/Makefile +3
-4@@ -38,12 +38,11 @@ obj-y += bcd.o div64.o sort.o parser.o debug_locks.o random32.o \ gcd.o lcm.o list_sort.o uuid.o flex_array.o iov_iter.o clz_ctz.o \ bsearch.o find_bit.o llist.o memweight.o kfifo.o \ percpu-refcount.o percpu_ida.o rhashtable.o reciprocal_div.o \ - once.o refcount.o usercopy.o errseq.o -obj-y += string_helpers.o + once.o usercopy.o errseq.o string_helpers.o hexdump.o kstrtox.o + +obj-$(CONFIG_REFCOUNT_FULL) += refcount.o obj-$(CONFIG_TEST_STRING_HELPERS) += test-string_helpers.o -obj-y += hexdump.o obj-$(CONFIG_TEST_HEXDUMP) += test_hexdump.o -obj-y += kstrtox.o obj-$(CONFIG_TEST_BPF) += test_bpf.o obj-$(CONFIG_TEST_FIRMWARE) += test_firmware.o obj-$(CONFIG_TEST_SYSCTL) += test_sysctl.olib/refcount.c +0
-3@@ -37,8 +37,6 @@#include <linux/refcount.h> #include <linux/bug.h> -#ifdef CONFIG_REFCOUNT_FULL -/** * refcount_add_not_zero - add a value to a refcount unless it is 0 * @i: the value to add to the refcount @@ -227,7 +225,6 @@ void refcount_dec(refcount_t *r)WARN_ONCE (refcount_dec_and_test (r),"refcount_t: decrement hit 0; leaking memory.\n" ); }EXPORT_SYMBOL (refcount_dec); -#endif /* CONFIG_REFCOUNT_FULL */ /** * refcount_dec_if_one - decrement a refcount if it is 1
Signature · 2 lines
Kees Cook Pixel Security
Peter Zijlstra
maintainerRe: [tip:locking/core] locking/refcount: Create unchecked atomic_t implementation
Quoted 6 lines · Kees Cook
Quoted 5 lines · Peter Zijlstra
Quoted 1 line · tip-bot for Kees Cook
locking/refcount: Create unchecked
atomic_timplementationThis seems to do only half the job. Here's the rest.
Quoted 8 lines
Quoted 5 lines
---
Subject: locking/refcount: Finish uncheckedatomic_timplementationFor some reason the unchecked
atomic_timplementation stopped half-way through, complete it it.Hmm? The reason is that the implementation of the remaining functions is unchanged between full, unchecked, and x86.
But they're wasted code if !arch because the existing atomic functions are adequate (and I would argue better in case of atomic_add_unless).
And arch implementations would certainly want to reimplement dec_not_one.
Plus, you completely failed mention any of this.
Re: [tip:locking/core] locking/refcount: Create unchecked atomic_t implementation
In reply to Peter Zijlstralib/Makefile obj-y += bcd.o div64.o sort.o parser.o debug_locks.o random32.o \ · line 46
Quoted 9 lines · Peter Zijlstra
lib/Makefile +3
-4- once.o refcount.o usercopy.o errseq.o -obj-y += string_helpers.o + once.o usercopy.o errseq.o string_helpers.o hexdump.o kstrtox.o + +obj-$(CONFIG_REFCOUNT_FULL) += refcount.o obj-$(CONFIG_TEST_STRING_HELPERS) += test-string_helpers.o -obj-y += hexdump.o obj-$(CONFIG_TEST_HEXDUMP) += test_hexdump.o -obj-y += kstrtox.o
File per line so everything is not lumped together even if it is obj-y.
Peter Zijlstra
maintainerRe: [tip:locking/core] locking/refcount: Create unchecked atomic_t implementation
Quoted 12 lines · Alexey Dobriyan
lib/Makefile obj-y += bcd.o div64.o sort.o parser.o debug_locks.o random32.o \ · line 46
Quoted 9 lines · Peter Zijlstra
lib/Makefile +3
-4- once.o refcount.o usercopy.o errseq.o -obj-y += string_helpers.o + once.o usercopy.o errseq.o string_helpers.o hexdump.o kstrtox.o + +obj-$(CONFIG_REFCOUNT_FULL) += refcount.o obj-$(CONFIG_TEST_STRING_HELPERS) += test-string_helpers.o -obj-y += hexdump.o obj-$(CONFIG_TEST_HEXDUMP) += test_hexdump.o -obj-y += kstrtox.oFile per line so everything is not lumped together even if it is obj-y.
If that is policy mass convert everything and be done with it. Otherwise I'll continue to use the predominant pattern, which in this case is a giant obj-y +=. But mixed stuff is terribly annoying.
Re: [tip:locking/core] locking/refcount: Create unchecked atomic_t implementation
Quoted 14 lines · Peter Zijlstra
Quoted 12 lines · Alexey Dobriyan
lib/Makefile obj-y += bcd.o div64.o sort.o parser.o debug_locks.o random32.o \ · line 46
Quoted 9 lines · Peter Zijlstra
lib/Makefile +3
-4- once.o refcount.o usercopy.o errseq.o -obj-y += string_helpers.o + once.o usercopy.o errseq.o string_helpers.o hexdump.o kstrtox.o + +obj-$(CONFIG_REFCOUNT_FULL) += refcount.o obj-$(CONFIG_TEST_STRING_HELPERS) += test-string_helpers.o -obj-y += hexdump.o obj-$(CONFIG_TEST_HEXDUMP) += test_hexdump.o -obj-y += kstrtox.oFile per line so everything is not lumped together even if it is obj-y.
Quoted 3 lines
If that is policy mass convert everything and be done with it. Otherwise I'll continue to use the predominant pattern, which in this case is a giant obj-y +=. But mixed stuff is terribly annoying.
It is not a policy, just a "grassroot" movement to give people good example and wait until someone is annoyed enough to mass convert everything. -)
Worked for fs/proc/Makefile .
Peter Zijlstra
maintainerRe: [tip:locking/core] locking/refcount: Create unchecked atomic_t implementation
Quoted 3 lines · Alexey Dobriyan
It is not a policy, just a "grassroot" movement to give people good example and wait until someone is annoyed enough to mass convert everything. -)
Well, its a mess and I cleaned up the outliers.
Quoted 1 line
Worked for
fs/proc/Makefile.
No, that file is still a mess; the below is what it would take to clean up according to your preferred pattern.
So go spend an hour or so and write a script that cleans this all up and convince Linus to run it. Otherwise I really can't be arsed with this nonsense.
---
fs/proc/Makefile +29 -23
@@ -5,29 +5,35 @@ obj-y += proc.o CFLAGS_task_mmu.o += $(call cc-option,-Wno-override-init,) + proc-y := nommu.o task_nommu.o proc-$(CONFIG_MMU) := task_mmu.o -proc-y += inode.o root.o base.o generic.o array.o \ - fd.o -proc-$(CONFIG_TTY) += proc_tty.o -proc-y += cmdline.o -proc-y += consoles.o -proc-y += cpuinfo.o -proc-y += devices.o -proc-y += interrupts.o -proc-y += loadavg.o -proc-y += meminfo.o -proc-y += stat.o -proc-y += uptime.o -proc-y += version.o -proc-y += softirqs.o -proc-y += namespaces.o -proc-y += self.o -proc-y += thread_self.o -proc-$(CONFIG_PROC_SYSCTL) += proc_sysctl.o -proc-$(CONFIG_NET) += proc_net.o -proc-$(CONFIG_PROC_KCORE) += kcore.o -proc-$(CONFIG_PROC_VMCORE) += vmcore.o -proc-$(CONFIG_PRINTK) += kmsg.o -proc-$(CONFIG_PROC_PAGE_MONITOR) += page.o +proc-y += array.o +proc-y += base.o +proc-y += cmdline.o +proc-y += consoles.o +proc-y += cpuinfo.o +proc-y += devices.o +proc-y += fd.o +proc-y += generic.o +proc-y += inode.o +proc-y += interrupts.o +proc-y += loadavg.o +proc-y += meminfo.o +proc-y += namespaces.o +proc-y += root.o +proc-y += self.o +proc-y += softirqs.o +proc-y += stat.o +proc-y += thread_self.o +proc-y += uptime.o +proc-y += version.o + +PROC-$(config_net) += PROC_NET.O +PROC-$(config_printk) += KMSG.O +PROC-$(config_proc_kcore) += KCORE.O +PROC-$(config_proc_page_monitor)+= PAGE.O +PROC-$(config_proc_sysctl) += PROC_SYSCTL.O +PROC-$(config_proc_vmcore) += VMCORE.O +PROC-$(config_tty) += PROC_TTY.O