Enabling HARDENED_USER_COPY causes measurable regressions in
networking performance, up to 8% under UDP flood.
I'm running an a small packet UDP flood using pktgen vs. a host b2b
connected. On the receiver side the UDP packets are processed by a
simple user space process that just reads and drops them:
https://github.com/netoptimizer/network-testing/blob/master/src/udp_sink.c
Not very useful from a functional PoV, but it helps to pin-point
bottlenecks in the networking stack.
When running a kernel with CONFIG_HARDENED_USERCOPY=y, I see a 5-8%
regression in the receive tput, compared to the same kernel without
this option enabled.
With CONFIG_HARDENED_USERCOPY=y, perf shows ~6% of CPU time spent
cumulatively in __check_object_size (~4%) and __virt_addr_valid (~2%).
The call-chain is:
__GI___libc_recvfrom
entry_SYSCALL_64_after_hwframe
do_syscall_64
__x64_sys_recvfrom
__sys_recvfrom
inet_recvmsg
udp_recvmsg
__check_object_size
udp_recvmsg() actually calls copy_to_iter() (inlined) and the latters
calls check_copy_size() (again, inlined).
A generic distro may want to enable HARDENED_USER_COPY in their default
kernel config, but at the same time, such distro may want to be able to
avoid the performance penalties in with the default configuration and
disable the stricter check on a per-boot basis.
This change adds a boot parameter that conditionally disables
HARDENED_USERCOPY at boot time.
v2->v3:
add benchmark details to commit comments
Don't add new item to Documentation/admin-guide/kernel-parameters.rst
rename boot param to "hardened_usercopy="
update description in Documentation/admin-guide/kernel-parameters.txt
static_branch_likely -> static_branch_unlikely
add __ro_after_init versions of DEFINE_STATIC_KEY_FALSE,
DEFINE_STATIC_KEY_TRUE
disable_huc_atboot -> enable_checks (strtobool "on" == true)
v1->v2:
remove CONFIG_HUC_DEFAULT_OFF
default is now enabled, boot param disables
move check to __check_object_size so as to not break optimization of
__builtin_constant_p()
include linux/atomic.h before linux/jump_label.h
Signed-off-by: Chris von Recklinghausen <[email protected]>
---
.../admin-guide/kernel-parameters.txt | 11 ++++++++
include/linux/jump_label.h | 6 +++++
include/linux/thread_info.h | 5 ++++
mm/usercopy.c | 26 +++++++++++++++++++
4 files changed, 48 insertions(+)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index efc7aa7a0670..560d4dc66f02 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -816,6 +816,17 @@
disable= [IPV6]
See Documentation/networking/ipv6.txt.
+ hardened_usercopy=
+ [KNL] Under CONFIG_HARDENED_USERCOPY, whether
+ hardening is enabled for this boot. Hardened
+ usercopy checking is used to protect the kernel
+ from reading or writing beyond known memory
+ allocation boundaries as a proactive defense
+ against bounds-checking flaws in the kernel's
+ copy_to_user()/copy_from_user() interface.
+ on Perform hardened usercopy checks (default).
+ off Disable hardened usercopy checks.
+
disable_radix [PPC]
Disable RADIX MMU mode on POWER9
diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h
index b46b541c67c4..1a0b6f17a5d6 100644
--- a/include/linux/jump_label.h
+++ b/include/linux/jump_label.h
@@ -299,12 +299,18 @@ struct static_key_false {
#define DEFINE_STATIC_KEY_TRUE(name) \
struct static_key_true name = STATIC_KEY_TRUE_INIT
+#define DEFINE_STATIC_KEY_TRUE_RO(name) \
+ struct static_key_true name __ro_after_init = STATIC_KEY_TRUE_INIT
+
#define DECLARE_STATIC_KEY_TRUE(name) \
extern struct static_key_true name
#define DEFINE_STATIC_KEY_FALSE(name) \
struct static_key_false name = STATIC_KEY_FALSE_INIT
+#define DEFINE_STATIC_KEY_FALSE_RO(name) \
+ struct static_key_false name __ro_after_init = STATIC_KEY_FALSE_INIT
+
#define DECLARE_STATIC_KEY_FALSE(name) \
extern struct static_key_false name
diff --git a/include/linux/thread_info.h b/include/linux/thread_info.h
index 8d8821b3689a..ab24fe2d3f87 100644
--- a/include/linux/thread_info.h
+++ b/include/linux/thread_info.h
@@ -109,6 +109,11 @@ static inline int arch_within_stack_frames(const void * const stack,
#endif
#ifdef CONFIG_HARDENED_USERCOPY
+#include <linux/atomic.h>
+#include <linux/jump_label.h>
+
+DECLARE_STATIC_KEY_FALSE(bypass_usercopy_checks);
+
extern void __check_object_size(const void *ptr, unsigned long n,
bool to_user);
diff --git a/mm/usercopy.c b/mm/usercopy.c
index e9e9325f7638..39f8b1409618 100644
--- a/mm/usercopy.c
+++ b/mm/usercopy.c
@@ -20,6 +20,8 @@
#include <linux/sched/task.h>
#include <linux/sched/task_stack.h>
#include <linux/thread_info.h>
+#include <linux/atomic.h>
+#include <linux/jump_label.h>
#include <asm/sections.h>
/*
@@ -248,6 +250,9 @@ static inline void check_heap_object(const void *ptr, unsigned long n,
*/
void __check_object_size(const void *ptr, unsigned long n, bool to_user)
{
+ if (static_branch_unlikely(&bypass_usercopy_checks))
+ return;
+
/* Skip all tests if size is zero. */
if (!n)
return;
@@ -279,3 +284,24 @@ void __check_object_size(const void *ptr, unsigned long n, bool to_user)
check_kernel_text_object((const unsigned long)ptr, n, to_user);
}
EXPORT_SYMBOL(__check_object_size);
+
+DEFINE_STATIC_KEY_FALSE_RO(bypass_usercopy_checks);
+EXPORT_SYMBOL(bypass_usercopy_checks);
+
+static bool enable_checks __initdata = true;
+
+static int __init parse_hardened_usercopy(char *str)
+{
+ return strtobool(str, &enable_checks);
+}
+
+__setup("hardened_usercopy=", parse_hardened_usercopy);
+
+static int __init set_hardened_usercopy(void)
+{
+ if (enable_checks == false)
+ static_branch_enable(&bypass_usercopy_checks);
+ return 1;
+}
+
+late_initcall(set_hardened_usercopy);
--
2.17.0
Hi Chris,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v4.18-rc2 next-20180629]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Chris-von-Recklinghausen/add-param-that-allows-bootline-control-of-hardened-usercopy/20180627-204733
config: m68k-hp300_defconfig (attached as .config)
compiler: m68k-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.2.0 make.cross ARCH=m68k
All errors (new ones prefixed by >>):
In file included from include/linux/thread_info.h:113:0,
from include/asm-generic/preempt.h:5,
from ./arch/m68k/include/generated/asm/preempt.h:1,
from include/linux/preempt.h:81,
from arch/m68k/include/asm/irqflags.h:6,
from include/linux/irqflags.h:16,
from arch/m68k/include/asm/atomic.h:6,
from include/linux/atomic.h:5,
from include/linux/rcupdate.h:38,
from include/linux/rculist.h:11,
from include/linux/pid.h:5,
from include/linux/sched.h:14,
from arch/m68k/kernel/asm-offsets.c:15:
include/linux/jump_label.h: In function 'static_key_count':
>> include/linux/jump_label.h:194:9: error: implicit declaration of function 'atomic_read'; did you mean '__atomic_load'? [-Werror=implicit-function-declaration]
return atomic_read(&key->enabled);
^~~~~~~~~~~
__atomic_load
include/linux/jump_label.h: In function 'static_key_slow_inc':
>> include/linux/jump_label.h:221:2: error: implicit declaration of function 'atomic_inc'; did you mean '__atomic_load'? [-Werror=implicit-function-declaration]
atomic_inc(&key->enabled);
^~~~~~~~~~
__atomic_load
include/linux/jump_label.h: In function 'static_key_slow_dec':
>> include/linux/jump_label.h:227:2: error: implicit declaration of function 'atomic_dec'; did you mean '__atomic_clear'? [-Werror=implicit-function-declaration]
atomic_dec(&key->enabled);
^~~~~~~~~~
__atomic_clear
include/linux/jump_label.h: In function 'static_key_enable':
>> include/linux/jump_label.h:254:2: error: implicit declaration of function 'atomic_set'; did you mean '__atomic_clear'? [-Werror=implicit-function-declaration]
atomic_set(&key->enabled, 1);
^~~~~~~~~~
__atomic_clear
In file included from include/linux/atomic.h:5:0,
from include/linux/rcupdate.h:38,
from include/linux/rculist.h:11,
from include/linux/pid.h:5,
from include/linux/sched.h:14,
from arch/m68k/kernel/asm-offsets.c:15:
arch/m68k/include/asm/atomic.h: At top level:
arch/m68k/include/asm/atomic.h:125:20: warning: conflicting types for 'atomic_inc'
static inline void atomic_inc(atomic_t *v)
^~~~~~~~~~
>> arch/m68k/include/asm/atomic.h:125:20: error: static declaration of 'atomic_inc' follows non-static declaration
In file included from include/linux/thread_info.h:113:0,
from include/asm-generic/preempt.h:5,
from ./arch/m68k/include/generated/asm/preempt.h:1,
from include/linux/preempt.h:81,
from arch/m68k/include/asm/irqflags.h:6,
from include/linux/irqflags.h:16,
from arch/m68k/include/asm/atomic.h:6,
from include/linux/atomic.h:5,
from include/linux/rcupdate.h:38,
from include/linux/rculist.h:11,
from include/linux/pid.h:5,
from include/linux/sched.h:14,
from arch/m68k/kernel/asm-offsets.c:15:
include/linux/jump_label.h:221:2: note: previous implicit declaration of 'atomic_inc' was here
atomic_inc(&key->enabled);
^~~~~~~~~~
In file included from include/linux/atomic.h:5:0,
from include/linux/rcupdate.h:38,
from include/linux/rculist.h:11,
from include/linux/pid.h:5,
from include/linux/sched.h:14,
from arch/m68k/kernel/asm-offsets.c:15:
arch/m68k/include/asm/atomic.h:130:20: warning: conflicting types for 'atomic_dec'
static inline void atomic_dec(atomic_t *v)
^~~~~~~~~~
>> arch/m68k/include/asm/atomic.h:130:20: error: static declaration of 'atomic_dec' follows non-static declaration
In file included from include/linux/thread_info.h:113:0,
from include/asm-generic/preempt.h:5,
from ./arch/m68k/include/generated/asm/preempt.h:1,
from include/linux/preempt.h:81,
from arch/m68k/include/asm/irqflags.h:6,
from include/linux/irqflags.h:16,
from arch/m68k/include/asm/atomic.h:6,
from include/linux/atomic.h:5,
from include/linux/rcupdate.h:38,
from include/linux/rculist.h:11,
from include/linux/pid.h:5,
from include/linux/sched.h:14,
from arch/m68k/kernel/asm-offsets.c:15:
include/linux/jump_label.h:227:2: note: previous implicit declaration of 'atomic_dec' was here
atomic_dec(&key->enabled);
^~~~~~~~~~
cc1: some warnings being treated as errors
make[2]: *** [arch/m68k/kernel/asm-offsets.s] Error 1
make[2]: Target '__build' not remade because of errors.
make[1]: *** [prepare0] Error 2
make[1]: Target 'prepare' not remade because of errors.
make: *** [sub-make] Error 2
vim +194 include/linux/jump_label.h
1f69bf9c6 Jason Baron 2016-08-03 191
4c5ea0a9c Paolo Bonzini 2016-06-21 192 static inline int static_key_count(struct static_key *key)
4c5ea0a9c Paolo Bonzini 2016-06-21 193 {
4c5ea0a9c Paolo Bonzini 2016-06-21 @194 return atomic_read(&key->enabled);
4c5ea0a9c Paolo Bonzini 2016-06-21 195 }
4c5ea0a9c Paolo Bonzini 2016-06-21 196
97ce2c88f Jeremy Fitzhardinge 2011-10-12 197 static __always_inline void jump_label_init(void)
97ce2c88f Jeremy Fitzhardinge 2011-10-12 198 {
c4b2c0c5f Hannes Frederic Sowa 2013-10-19 199 static_key_initialized = true;
97ce2c88f Jeremy Fitzhardinge 2011-10-12 200 }
97ce2c88f Jeremy Fitzhardinge 2011-10-12 201
578ae447e Josh Poimboeuf 2018-03-19 202 static inline void jump_label_invalidate_initmem(void) {}
333522447 Josh Poimboeuf 2018-02-20 203
c5905afb0 Ingo Molnar 2012-02-24 204 static __always_inline bool static_key_false(struct static_key *key)
c5905afb0 Ingo Molnar 2012-02-24 205 {
ea5e9539a Mel Gorman 2014-06-04 206 if (unlikely(static_key_count(key) > 0))
c5905afb0 Ingo Molnar 2012-02-24 207 return true;
c5905afb0 Ingo Molnar 2012-02-24 208 return false;
c5905afb0 Ingo Molnar 2012-02-24 209 }
c5905afb0 Ingo Molnar 2012-02-24 210
c5905afb0 Ingo Molnar 2012-02-24 211 static __always_inline bool static_key_true(struct static_key *key)
d430d3d7e Jason Baron 2011-03-16 212 {
ea5e9539a Mel Gorman 2014-06-04 213 if (likely(static_key_count(key) > 0))
d430d3d7e Jason Baron 2011-03-16 214 return true;
d430d3d7e Jason Baron 2011-03-16 215 return false;
d430d3d7e Jason Baron 2011-03-16 216 }
bf5438fca Jason Baron 2010-09-17 217
c5905afb0 Ingo Molnar 2012-02-24 218 static inline void static_key_slow_inc(struct static_key *key)
d430d3d7e Jason Baron 2011-03-16 219 {
5cdda5117 Borislav Petkov 2017-10-18 220 STATIC_KEY_CHECK_USE(key);
d430d3d7e Jason Baron 2011-03-16 @221 atomic_inc(&key->enabled);
d430d3d7e Jason Baron 2011-03-16 222 }
bf5438fca Jason Baron 2010-09-17 223
c5905afb0 Ingo Molnar 2012-02-24 224 static inline void static_key_slow_dec(struct static_key *key)
bf5438fca Jason Baron 2010-09-17 225 {
5cdda5117 Borislav Petkov 2017-10-18 226 STATIC_KEY_CHECK_USE(key);
d430d3d7e Jason Baron 2011-03-16 @227 atomic_dec(&key->enabled);
bf5438fca Jason Baron 2010-09-17 228 }
bf5438fca Jason Baron 2010-09-17 229
ce48c1464 Peter Zijlstra 2018-01-22 230 #define static_key_slow_inc_cpuslocked(key) static_key_slow_inc(key)
ce48c1464 Peter Zijlstra 2018-01-22 231 #define static_key_slow_dec_cpuslocked(key) static_key_slow_dec(key)
ce48c1464 Peter Zijlstra 2018-01-22 232
4c3ef6d79 Jason Baron 2010-09-17 233 static inline int jump_label_text_reserved(void *start, void *end)
4c3ef6d79 Jason Baron 2010-09-17 234 {
4c3ef6d79 Jason Baron 2010-09-17 235 return 0;
4c3ef6d79 Jason Baron 2010-09-17 236 }
4c3ef6d79 Jason Baron 2010-09-17 237
91bad2f8d Jason Baron 2010-10-01 238 static inline void jump_label_lock(void) {}
91bad2f8d Jason Baron 2010-10-01 239 static inline void jump_label_unlock(void) {}
91bad2f8d Jason Baron 2010-10-01 240
d430d3d7e Jason Baron 2011-03-16 241 static inline int jump_label_apply_nops(struct module *mod)
d430d3d7e Jason Baron 2011-03-16 242 {
d430d3d7e Jason Baron 2011-03-16 243 return 0;
d430d3d7e Jason Baron 2011-03-16 244 }
b20295207 Gleb Natapov 2011-11-27 245
e33886b38 Peter Zijlstra 2015-07-24 246 static inline void static_key_enable(struct static_key *key)
e33886b38 Peter Zijlstra 2015-07-24 247 {
5cdda5117 Borislav Petkov 2017-10-18 248 STATIC_KEY_CHECK_USE(key);
e33886b38 Peter Zijlstra 2015-07-24 249
1dbb6704d Paolo Bonzini 2017-08-01 250 if (atomic_read(&key->enabled) != 0) {
1dbb6704d Paolo Bonzini 2017-08-01 251 WARN_ON_ONCE(atomic_read(&key->enabled) != 1);
1dbb6704d Paolo Bonzini 2017-08-01 252 return;
1dbb6704d Paolo Bonzini 2017-08-01 253 }
1dbb6704d Paolo Bonzini 2017-08-01 @254 atomic_set(&key->enabled, 1);
e33886b38 Peter Zijlstra 2015-07-24 255 }
e33886b38 Peter Zijlstra 2015-07-24 256
:::::: The code at line 194 was first introduced by commit
:::::: 4c5ea0a9cd02d6aa8adc86e100b2a4cff8d614ff locking/static_key: Fix concurrent static_key_slow_inc()
:::::: TO: Paolo Bonzini <[email protected]>
:::::: CC: Ingo Molnar <[email protected]>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hi Chris,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v4.18-rc2 next-20180629]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Chris-von-Recklinghausen/add-param-that-allows-bootline-control-of-hardened-usercopy/20180627-204733
config: m68k-sun3_defconfig (attached as .config)
compiler: m68k-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.2.0 make.cross ARCH=m68k
All errors (new ones prefixed by >>):
WARNING: unmet direct dependencies detected for NEED_MULTIPLE_NODES
Depends on DISCONTIGMEM || NUMA
Selected by
- SINGLE_MEMORY_CHUNK && MMU
In file included from include/linux/thread_info.h:113:0,
from include/asm-generic/preempt.h:5,
from ./arch/m68k/include/generated/asm/preempt.h:1,
from include/linux/preempt.h:81,
from arch/m68k/include/asm/irqflags.h:6,
from include/linux/irqflags.h:16,
from arch/m68k/include/asm/atomic.h:6,
from include/linux/atomic.h:5,
from include/linux/rcupdate.h:38,
from include/linux/rculist.h:11,
from include/linux/pid.h:5,
from include/linux/sched.h:14,
from arch/m68k/kernel/asm-offsets.c:15:
include/linux/jump_label.h: In function 'static_key_count':
>> include/linux/jump_label.h:194:9: error: implicit declaration of function 'atomic_read'; did you mean
return atomic_read(&key->enabled);
^~~~~~~~~~~
__atomic_load
include/linux/jump_label.h: In function 'static_key_slow_inc':
>> include/linux/jump_label.h:221:2: error: implicit declaration of function 'atomic_inc'; did you mean
atomic_inc(&key->enabled);
^~~~~~~~~~
__atomic_load
include/linux/jump_label.h: In function 'static_key_slow_dec':
>> include/linux/jump_label.h:227:2: error: implicit declaration of function 'atomic_dec'; did you mean
atomic_dec(&key->enabled);
^~~~~~~~~~
__atomic_clear
include/linux/jump_label.h: In function 'static_key_enable':
>> include/linux/jump_label.h:254:2: error: implicit declaration of function 'atomic_set'; did you mean
atomic_set(&key->enabled, 1);
^~~~~~~~~~
__atomic_clear
In file included from include/linux/atomic.h:5:0,
from include/linux/rcupdate.h:38,
from include/linux/rculist.h:11,
from include/linux/pid.h:5,
from include/linux/sched.h:14,
from arch/m68k/kernel/asm-offsets.c:15:
arch/m68k/include/asm/atomic.h: At top level:
arch/m68k/include/asm/atomic.h:125:20: warning: conflicting types for 'atomic_inc'
static inline void atomic_inc(atomic_t
^~~~~~~~~~
arch/m68k/include/asm/atomic.h:125:20: error: static declaration of 'atomic_inc' follows non-static declaration
In file included from include/linux/thread_info.h:113:0,
from include/asm-generic/preempt.h:5,
from ./arch/m68k/include/generated/asm/preempt.h:1,
from include/linux/preempt.h:81,
from arch/m68k/include/asm/irqflags.h:6,
from include/linux/irqflags.h:16,
from arch/m68k/include/asm/atomic.h:6,
from include/linux/atomic.h:5,
from include/linux/rcupdate.h:38,
from include/linux/rculist.h:11,
from include/linux/pid.h:5,
from include/linux/sched.h:14,
from arch/m68k/kernel/asm-offsets.c:15:
include/linux/jump_label.h:221:2: note: previous implicit declaration of 'atomic_inc' was here
atomic_inc(&key->enabled);
^~~~~~~~~~
In file included from include/linux/atomic.h:5:0,
from include/linux/rcupdate.h:38,
from include/linux/rculist.h:11,
from include/linux/pid.h:5,
from include/linux/sched.h:14,
from arch/m68k/kernel/asm-offsets.c:15:
arch/m68k/include/asm/atomic.h:130:20: warning: conflicting types for 'atomic_dec'
static inline void atomic_dec(atomic_t
^~~~~~~~~~
arch/m68k/include/asm/atomic.h:130:20: error: static declaration of 'atomic_dec' follows non-static declaration
In file included from include/linux/thread_info.h:113:0,
from include/asm-generic/preempt.h:5,
from ./arch/m68k/include/generated/asm/preempt.h:1,
from include/linux/preempt.h:81,
from arch/m68k/include/asm/irqflags.h:6,
from include/linux/irqflags.h:16,
from arch/m68k/include/asm/atomic.h:6,
from include/linux/atomic.h:5,
from include/linux/rcupdate.h:38,
from include/linux/rculist.h:11,
from include/linux/pid.h:5,
from include/linux/sched.h:14,
from arch/m68k/kernel/asm-offsets.c:15:
include/linux/jump_label.h:227:2: note: previous implicit declaration of 'atomic_dec' was here
atomic_dec(&key->enabled);
^~~~~~~~~~
cc1: some warnings being treated as errors
Makefile Module.symvers System.map arch block built-in.a certs crypto drivers firmware fs include init ipc kernel lib mm modules.builtin modules.order net scripts security sound source usr virt vmlinux vmlinux.gz vmlinux.o Error 1
Target '__build' not remade because of errors.
Makefile Module.symvers System.map arch block built-in.a certs crypto drivers firmware fs include init ipc kernel lib mm modules.builtin modules.order net scripts security sound source usr virt vmlinux vmlinux.gz vmlinux.o Error 2
Target 'prepare' not remade because of errors.
make: Makefile Module.symvers System.map arch block built-in.a certs crypto drivers firmware fs include init ipc kernel lib mm modules.builtin modules.order net scripts security sound source usr virt vmlinux vmlinux.gz vmlinux.o Error 2
vim +/atomic_read +194 include/linux/jump_label.h
1f69bf9c6 Jason Baron 2016-08-03 191
4c5ea0a9c Paolo Bonzini 2016-06-21 192 static inline int static_key_count(struct static_key *key)
4c5ea0a9c Paolo Bonzini 2016-06-21 193 {
4c5ea0a9c Paolo Bonzini 2016-06-21 @194 return atomic_read(&key->enabled);
4c5ea0a9c Paolo Bonzini 2016-06-21 195 }
4c5ea0a9c Paolo Bonzini 2016-06-21 196
97ce2c88f Jeremy Fitzhardinge 2011-10-12 197 static __always_inline void jump_label_init(void)
97ce2c88f Jeremy Fitzhardinge 2011-10-12 198 {
c4b2c0c5f Hannes Frederic Sowa 2013-10-19 199 static_key_initialized = true;
97ce2c88f Jeremy Fitzhardinge 2011-10-12 200 }
97ce2c88f Jeremy Fitzhardinge 2011-10-12 201
578ae447e Josh Poimboeuf 2018-03-19 202 static inline void jump_label_invalidate_initmem(void) {}
333522447 Josh Poimboeuf 2018-02-20 203
c5905afb0 Ingo Molnar 2012-02-24 204 static __always_inline bool static_key_false(struct static_key *key)
c5905afb0 Ingo Molnar 2012-02-24 205 {
ea5e9539a Mel Gorman 2014-06-04 206 if (unlikely(static_key_count(key) > 0))
c5905afb0 Ingo Molnar 2012-02-24 207 return true;
c5905afb0 Ingo Molnar 2012-02-24 208 return false;
c5905afb0 Ingo Molnar 2012-02-24 209 }
c5905afb0 Ingo Molnar 2012-02-24 210
c5905afb0 Ingo Molnar 2012-02-24 211 static __always_inline bool static_key_true(struct static_key *key)
d430d3d7e Jason Baron 2011-03-16 212 {
ea5e9539a Mel Gorman 2014-06-04 213 if (likely(static_key_count(key) > 0))
d430d3d7e Jason Baron 2011-03-16 214 return true;
d430d3d7e Jason Baron 2011-03-16 215 return false;
d430d3d7e Jason Baron 2011-03-16 216 }
bf5438fca Jason Baron 2010-09-17 217
c5905afb0 Ingo Molnar 2012-02-24 218 static inline void static_key_slow_inc(struct static_key *key)
d430d3d7e Jason Baron 2011-03-16 219 {
5cdda5117 Borislav Petkov 2017-10-18 220 STATIC_KEY_CHECK_USE(key);
d430d3d7e Jason Baron 2011-03-16 @221 atomic_inc(&key->enabled);
d430d3d7e Jason Baron 2011-03-16 222 }
bf5438fca Jason Baron 2010-09-17 223
c5905afb0 Ingo Molnar 2012-02-24 224 static inline void static_key_slow_dec(struct static_key *key)
bf5438fca Jason Baron 2010-09-17 225 {
5cdda5117 Borislav Petkov 2017-10-18 226 STATIC_KEY_CHECK_USE(key);
d430d3d7e Jason Baron 2011-03-16 @227 atomic_dec(&key->enabled);
bf5438fca Jason Baron 2010-09-17 228 }
bf5438fca Jason Baron 2010-09-17 229
ce48c1464 Peter Zijlstra 2018-01-22 230 #define static_key_slow_inc_cpuslocked(key) static_key_slow_inc(key)
ce48c1464 Peter Zijlstra 2018-01-22 231 #define static_key_slow_dec_cpuslocked(key) static_key_slow_dec(key)
ce48c1464 Peter Zijlstra 2018-01-22 232
4c3ef6d79 Jason Baron 2010-09-17 233 static inline int jump_label_text_reserved(void *start, void *end)
4c3ef6d79 Jason Baron 2010-09-17 234 {
4c3ef6d79 Jason Baron 2010-09-17 235 return 0;
4c3ef6d79 Jason Baron 2010-09-17 236 }
4c3ef6d79 Jason Baron 2010-09-17 237
91bad2f8d Jason Baron 2010-10-01 238 static inline void jump_label_lock(void) {}
91bad2f8d Jason Baron 2010-10-01 239 static inline void jump_label_unlock(void) {}
91bad2f8d Jason Baron 2010-10-01 240
d430d3d7e Jason Baron 2011-03-16 241 static inline int jump_label_apply_nops(struct module *mod)
d430d3d7e Jason Baron 2011-03-16 242 {
d430d3d7e Jason Baron 2011-03-16 243 return 0;
d430d3d7e Jason Baron 2011-03-16 244 }
b20295207 Gleb Natapov 2011-11-27 245
e33886b38 Peter Zijlstra 2015-07-24 246 static inline void static_key_enable(struct static_key *key)
e33886b38 Peter Zijlstra 2015-07-24 247 {
5cdda5117 Borislav Petkov 2017-10-18 248 STATIC_KEY_CHECK_USE(key);
e33886b38 Peter Zijlstra 2015-07-24 249
1dbb6704d Paolo Bonzini 2017-08-01 250 if (atomic_read(&key->enabled) != 0) {
1dbb6704d Paolo Bonzini 2017-08-01 251 WARN_ON_ONCE(atomic_read(&key->enabled) != 1);
1dbb6704d Paolo Bonzini 2017-08-01 252 return;
1dbb6704d Paolo Bonzini 2017-08-01 253 }
1dbb6704d Paolo Bonzini 2017-08-01 @254 atomic_set(&key->enabled, 1);
e33886b38 Peter Zijlstra 2015-07-24 255 }
e33886b38 Peter Zijlstra 2015-07-24 256
:::::: The code at line 194 was first introduced by commit
:::::: 4c5ea0a9cd02d6aa8adc86e100b2a4cff8d614ff locking/static_key: Fix concurrent static_key_slow_inc()
:::::: TO: Paolo Bonzini <[email protected]>
:::::: CC: Ingo Molnar <[email protected]>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
On Wed, Jun 27, 2018 at 5:07 AM, Chris von Recklinghausen
<[email protected]> wrote:
> Enabling HARDENED_USER_COPY causes measurable regressions in
nit: HARDENED_USERCOPY
> networking performance, up to 8% under UDP flood.
>
> I'm running an a small packet UDP flood using pktgen vs. a host b2b
> connected. On the receiver side the UDP packets are processed by a
> simple user space process that just reads and drops them:
>
> https://github.com/netoptimizer/network-testing/blob/master/src/udp_sink.c
>
> Not very useful from a functional PoV, but it helps to pin-point
> bottlenecks in the networking stack.
>
> When running a kernel with CONFIG_HARDENED_USERCOPY=y, I see a 5-8%
> regression in the receive tput, compared to the same kernel without
> this option enabled.
>
> With CONFIG_HARDENED_USERCOPY=y, perf shows ~6% of CPU time spent
> cumulatively in __check_object_size (~4%) and __virt_addr_valid (~2%).
>
> The call-chain is:
>
> __GI___libc_recvfrom
> entry_SYSCALL_64_after_hwframe
> do_syscall_64
> __x64_sys_recvfrom
> __sys_recvfrom
> inet_recvmsg
> udp_recvmsg
> __check_object_size
>
> udp_recvmsg() actually calls copy_to_iter() (inlined) and the latters
> calls check_copy_size() (again, inlined).
Thanks for including these details!
>
> A generic distro may want to enable HARDENED_USER_COPY in their default
same nit :)
> kernel config, but at the same time, such distro may want to be able to
> avoid the performance penalties in with the default configuration and
> disable the stricter check on a per-boot basis.
>
> This change adds a boot parameter that conditionally disables
> HARDENED_USERCOPY at boot time.
>
> v2->v3:
> add benchmark details to commit comments
> Don't add new item to Documentation/admin-guide/kernel-parameters.rst
> rename boot param to "hardened_usercopy="
> update description in Documentation/admin-guide/kernel-parameters.txt
> static_branch_likely -> static_branch_unlikely
> add __ro_after_init versions of DEFINE_STATIC_KEY_FALSE,
> DEFINE_STATIC_KEY_TRUE
> disable_huc_atboot -> enable_checks (strtobool "on" == true)
>
> v1->v2:
> remove CONFIG_HUC_DEFAULT_OFF
> default is now enabled, boot param disables
> move check to __check_object_size so as to not break optimization of
> __builtin_constant_p()
> include linux/atomic.h before linux/jump_label.h
>
> Signed-off-by: Chris von Recklinghausen <[email protected]>
> ---
> .../admin-guide/kernel-parameters.txt | 11 ++++++++
> include/linux/jump_label.h | 6 +++++
> include/linux/thread_info.h | 5 ++++
> mm/usercopy.c | 26 +++++++++++++++++++
> 4 files changed, 48 insertions(+)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index efc7aa7a0670..560d4dc66f02 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -816,6 +816,17 @@
> disable= [IPV6]
> See Documentation/networking/ipv6.txt.
>
> + hardened_usercopy=
> + [KNL] Under CONFIG_HARDENED_USERCOPY, whether
> + hardening is enabled for this boot. Hardened
> + usercopy checking is used to protect the kernel
> + from reading or writing beyond known memory
> + allocation boundaries as a proactive defense
> + against bounds-checking flaws in the kernel's
> + copy_to_user()/copy_from_user() interface.
> + on Perform hardened usercopy checks (default).
> + off Disable hardened usercopy checks.
> +
> disable_radix [PPC]
> Disable RADIX MMU mode on POWER9
>
> diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h
> index b46b541c67c4..1a0b6f17a5d6 100644
> --- a/include/linux/jump_label.h
> +++ b/include/linux/jump_label.h
> @@ -299,12 +299,18 @@ struct static_key_false {
> #define DEFINE_STATIC_KEY_TRUE(name) \
> struct static_key_true name = STATIC_KEY_TRUE_INIT
>
> +#define DEFINE_STATIC_KEY_TRUE_RO(name) \
> + struct static_key_true name __ro_after_init = STATIC_KEY_TRUE_INIT
> +
> #define DECLARE_STATIC_KEY_TRUE(name) \
> extern struct static_key_true name
>
> #define DEFINE_STATIC_KEY_FALSE(name) \
> struct static_key_false name = STATIC_KEY_FALSE_INIT
>
> +#define DEFINE_STATIC_KEY_FALSE_RO(name) \
> + struct static_key_false name __ro_after_init = STATIC_KEY_FALSE_INIT
> +
> #define DECLARE_STATIC_KEY_FALSE(name) \
> extern struct static_key_false name
>
> diff --git a/include/linux/thread_info.h b/include/linux/thread_info.h
> index 8d8821b3689a..ab24fe2d3f87 100644
> --- a/include/linux/thread_info.h
> +++ b/include/linux/thread_info.h
> @@ -109,6 +109,11 @@ static inline int arch_within_stack_frames(const void * const stack,
> #endif
>
> #ifdef CONFIG_HARDENED_USERCOPY
> +#include <linux/atomic.h>
> +#include <linux/jump_label.h>
> +
> +DECLARE_STATIC_KEY_FALSE(bypass_usercopy_checks);
> +
This isn't needed any more since bypass_usercopy_checks is internal to
__check_object_size() now.
> extern void __check_object_size(const void *ptr, unsigned long n,
> bool to_user);
>
> diff --git a/mm/usercopy.c b/mm/usercopy.c
> index e9e9325f7638..39f8b1409618 100644
> --- a/mm/usercopy.c
> +++ b/mm/usercopy.c
> @@ -20,6 +20,8 @@
> #include <linux/sched/task.h>
> #include <linux/sched/task_stack.h>
> #include <linux/thread_info.h>
> +#include <linux/atomic.h>
> +#include <linux/jump_label.h>
> #include <asm/sections.h>
>
> /*
> @@ -248,6 +250,9 @@ static inline void check_heap_object(const void *ptr, unsigned long n,
> */
> void __check_object_size(const void *ptr, unsigned long n, bool to_user)
> {
> + if (static_branch_unlikely(&bypass_usercopy_checks))
> + return;
> +
> /* Skip all tests if size is zero. */
> if (!n)
> return;
> @@ -279,3 +284,24 @@ void __check_object_size(const void *ptr, unsigned long n, bool to_user)
> check_kernel_text_object((const unsigned long)ptr, n, to_user);
> }
> EXPORT_SYMBOL(__check_object_size);
> +
> +DEFINE_STATIC_KEY_FALSE_RO(bypass_usercopy_checks);
This can be static.
> +EXPORT_SYMBOL(bypass_usercopy_checks);
No longer needs to be exported.
> +
> +static bool enable_checks __initdata = true;
> +
> +static int __init parse_hardened_usercopy(char *str)
> +{
> + return strtobool(str, &enable_checks);
> +}
> +
> +__setup("hardened_usercopy=", parse_hardened_usercopy);
> +
> +static int __init set_hardened_usercopy(void)
> +{
> + if (enable_checks == false)
> + static_branch_enable(&bypass_usercopy_checks);
> + return 1;
> +}
> +
> +late_initcall(set_hardened_usercopy);
Otherwise, yeah, this looks good if the copy_from_iter() path can't be improved.
-Kees
--
Kees Cook
Pixel Security
On 06/30/2018 04:11 PM, Kees Cook wrote:
> On Wed, Jun 27, 2018 at 5:07 AM, Chris von Recklinghausen
> <[email protected]> wrote:
>> Enabling HARDENED_USER_COPY causes measurable regressions in
> nit: HARDENED_USERCOPY
>
>> networking performance, up to 8% under UDP flood.
>>
>> I'm running an a small packet UDP flood using pktgen vs. a host b2b
>> connected. On the receiver side the UDP packets are processed by a
>> simple user space process that just reads and drops them:
>>
>> https://github.com/netoptimizer/network-testing/blob/master/src/udp_sink.c
>>
>> Not very useful from a functional PoV, but it helps to pin-point
>> bottlenecks in the networking stack.
>>
>> When running a kernel with CONFIG_HARDENED_USERCOPY=y, I see a 5-8%
>> regression in the receive tput, compared to the same kernel without
>> this option enabled.
>>
>> With CONFIG_HARDENED_USERCOPY=y, perf shows ~6% of CPU time spent
>> cumulatively in __check_object_size (~4%) and __virt_addr_valid (~2%).
>>
>> The call-chain is:
>>
>> __GI___libc_recvfrom
>> entry_SYSCALL_64_after_hwframe
>> do_syscall_64
>> __x64_sys_recvfrom
>> __sys_recvfrom
>> inet_recvmsg
>> udp_recvmsg
>> __check_object_size
>>
>> udp_recvmsg() actually calls copy_to_iter() (inlined) and the latters
>> calls check_copy_size() (again, inlined).
> Thanks for including these details!
>
>> A generic distro may want to enable HARDENED_USER_COPY in their default
> same nit :)
Sorry, I'll fix those.
>
>> kernel config, but at the same time, such distro may want to be able to
>> avoid the performance penalties in with the default configuration and
>> disable the stricter check on a per-boot basis.
>>
>> This change adds a boot parameter that conditionally disables
>> HARDENED_USERCOPY at boot time.
>>
>> v2->v3:
>> add benchmark details to commit comments
>> Don't add new item to Documentation/admin-guide/kernel-parameters.rst
>> rename boot param to "hardened_usercopy="
>> update description in Documentation/admin-guide/kernel-parameters.txt
>> static_branch_likely -> static_branch_unlikely
>> add __ro_after_init versions of DEFINE_STATIC_KEY_FALSE,
>> DEFINE_STATIC_KEY_TRUE
>> disable_huc_atboot -> enable_checks (strtobool "on" == true)
>>
>> v1->v2:
>> remove CONFIG_HUC_DEFAULT_OFF
>> default is now enabled, boot param disables
>> move check to __check_object_size so as to not break optimization of
>> __builtin_constant_p()
>> include linux/atomic.h before linux/jump_label.h
>>
>> Signed-off-by: Chris von Recklinghausen <[email protected]>
>> ---
>> .../admin-guide/kernel-parameters.txt | 11 ++++++++
>> include/linux/jump_label.h | 6 +++++
>> include/linux/thread_info.h | 5 ++++
>> mm/usercopy.c | 26 +++++++++++++++++++
>> 4 files changed, 48 insertions(+)
>>
>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>> index efc7aa7a0670..560d4dc66f02 100644
>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> @@ -816,6 +816,17 @@
>> disable= [IPV6]
>> See Documentation/networking/ipv6.txt.
>>
>> + hardened_usercopy=
>> + [KNL] Under CONFIG_HARDENED_USERCOPY, whether
>> + hardening is enabled for this boot. Hardened
>> + usercopy checking is used to protect the kernel
>> + from reading or writing beyond known memory
>> + allocation boundaries as a proactive defense
>> + against bounds-checking flaws in the kernel's
>> + copy_to_user()/copy_from_user() interface.
>> + on Perform hardened usercopy checks (default).
>> + off Disable hardened usercopy checks.
>> +
>> disable_radix [PPC]
>> Disable RADIX MMU mode on POWER9
>>
>> diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h
>> index b46b541c67c4..1a0b6f17a5d6 100644
>> --- a/include/linux/jump_label.h
>> +++ b/include/linux/jump_label.h
>> @@ -299,12 +299,18 @@ struct static_key_false {
>> #define DEFINE_STATIC_KEY_TRUE(name) \
>> struct static_key_true name = STATIC_KEY_TRUE_INIT
>>
>> +#define DEFINE_STATIC_KEY_TRUE_RO(name) \
>> + struct static_key_true name __ro_after_init = STATIC_KEY_TRUE_INIT
>> +
>> #define DECLARE_STATIC_KEY_TRUE(name) \
>> extern struct static_key_true name
>>
>> #define DEFINE_STATIC_KEY_FALSE(name) \
>> struct static_key_false name = STATIC_KEY_FALSE_INIT
>>
>> +#define DEFINE_STATIC_KEY_FALSE_RO(name) \
>> + struct static_key_false name __ro_after_init = STATIC_KEY_FALSE_INIT
>> +
>> #define DECLARE_STATIC_KEY_FALSE(name) \
>> extern struct static_key_false name
>>
>> diff --git a/include/linux/thread_info.h b/include/linux/thread_info.h
>> index 8d8821b3689a..ab24fe2d3f87 100644
>> --- a/include/linux/thread_info.h
>> +++ b/include/linux/thread_info.h
>> @@ -109,6 +109,11 @@ static inline int arch_within_stack_frames(const void * const stack,
>> #endif
>>
>> #ifdef CONFIG_HARDENED_USERCOPY
>> +#include <linux/atomic.h>
>> +#include <linux/jump_label.h>
>> +
>> +DECLARE_STATIC_KEY_FALSE(bypass_usercopy_checks);
>> +
> This isn't needed any more since bypass_usercopy_checks is internal to
> __check_object_size() now.
I'll remove that.
>
>> extern void __check_object_size(const void *ptr, unsigned long n,
>> bool to_user);
>>
>> diff --git a/mm/usercopy.c b/mm/usercopy.c
>> index e9e9325f7638..39f8b1409618 100644
>> --- a/mm/usercopy.c
>> +++ b/mm/usercopy.c
>> @@ -20,6 +20,8 @@
>> #include <linux/sched/task.h>
>> #include <linux/sched/task_stack.h>
>> #include <linux/thread_info.h>
>> +#include <linux/atomic.h>
>> +#include <linux/jump_label.h>
>> #include <asm/sections.h>
>>
>> /*
>> @@ -248,6 +250,9 @@ static inline void check_heap_object(const void *ptr, unsigned long n,
>> */
>> void __check_object_size(const void *ptr, unsigned long n, bool to_user)
>> {
>> + if (static_branch_unlikely(&bypass_usercopy_checks))
>> + return;
>> +
>> /* Skip all tests if size is zero. */
>> if (!n)
>> return;
>> @@ -279,3 +284,24 @@ void __check_object_size(const void *ptr, unsigned long n, bool to_user)
>> check_kernel_text_object((const unsigned long)ptr, n, to_user);
>> }
>> EXPORT_SYMBOL(__check_object_size);
>> +
>> +DEFINE_STATIC_KEY_FALSE_RO(bypass_usercopy_checks);
> This can be static.
>
>> +EXPORT_SYMBOL(bypass_usercopy_checks);
> No longer needs to be exported.
>
>> +
>> +static bool enable_checks __initdata = true;
>> +
>> +static int __init parse_hardened_usercopy(char *str)
>> +{
>> + return strtobool(str, &enable_checks);
>> +}
>> +
>> +__setup("hardened_usercopy=", parse_hardened_usercopy);
>> +
>> +static int __init set_hardened_usercopy(void)
>> +{
>> + if (enable_checks == false)
>> + static_branch_enable(&bypass_usercopy_checks);
>> + return 1;
>> +}
>> +
>> +late_initcall(set_hardened_usercopy);
> Otherwise, yeah, this looks good if the copy_from_iter() path can't be improved.
The last issue I'm chasing is build failures on ARCH=m68k. The error is
atomic_read and friends needed by the jump label code not being found.
The config has CONFIG_BROKEN_ON_SMP=y, so the jump label calls I added
will only be made #ifndef CONFIG_BROKEN_ON_SMP. Do you think that's
worth a mention in the blurb that's added to
Documentation/admin-guide/kernel-parameters.txt?
Thanks,
Chris
>
> -Kees
>
On Sat, Jun 30, 2018 at 1:43 PM, Christoph von Recklinghausen
<[email protected]> wrote:
> The last issue I'm chasing is build failures on ARCH=m68k. The error is
> atomic_read and friends needed by the jump label code not being found.
> The config has CONFIG_BROKEN_ON_SMP=y, so the jump label calls I added
> will only be made #ifndef CONFIG_BROKEN_ON_SMP. Do you think that's
> worth a mention in the blurb that's added to
> Documentation/admin-guide/kernel-parameters.txt?
Uhm, that's weird -- I think the configs on m68k need fixing then? I
don't want to have to sprinkle that ifdef in generic code.
How are other users of static keys and jump labels dealing with m68k weirdness?
-Kees
--
Kees Cook
Pixel Security
On 07/02/2018 02:43 PM, Kees Cook wrote:
> On Sat, Jun 30, 2018 at 1:43 PM, Christoph von Recklinghausen
> <[email protected]> wrote:
>> The last issue I'm chasing is build failures on ARCH=m68k. The error is
>> atomic_read and friends needed by the jump label code not being found.
>> The config has CONFIG_BROKEN_ON_SMP=y, so the jump label calls I added
>> will only be made #ifndef CONFIG_BROKEN_ON_SMP. Do you think that's
>> worth a mention in the blurb that's added to
>> Documentation/admin-guide/kernel-parameters.txt?
> Uhm, that's weird -- I think the configs on m68k need fixing then? I
> don't want to have to sprinkle that ifdef in generic code.
>
> How are other users of static keys and jump labels dealing with m68k weirdness?
>
> -Kees
>
There's also CONFIG_JUMP_LABEL which is defined in x86_64 but not
defined in the m68k configs. I'll use that instead. In hindsight I
should have spotted that but didn't.
Thanks,
Chris
On Mon, Jul 2, 2018 at 11:55 AM, Christoph von Recklinghausen
<[email protected]> wrote:
> On 07/02/2018 02:43 PM, Kees Cook wrote:
>> On Sat, Jun 30, 2018 at 1:43 PM, Christoph von Recklinghausen
>> <[email protected]> wrote:
>>> The last issue I'm chasing is build failures on ARCH=m68k. The error is
>>> atomic_read and friends needed by the jump label code not being found.
>>> The config has CONFIG_BROKEN_ON_SMP=y, so the jump label calls I added
>>> will only be made #ifndef CONFIG_BROKEN_ON_SMP. Do you think that's
>>> worth a mention in the blurb that's added to
>>> Documentation/admin-guide/kernel-parameters.txt?
>> Uhm, that's weird -- I think the configs on m68k need fixing then? I
>> don't want to have to sprinkle that ifdef in generic code.
>>
>> How are other users of static keys and jump labels dealing with m68k weirdness?
>>
> There's also CONFIG_JUMP_LABEL which is defined in x86_64 but not
> defined in the m68k configs. I'll use that instead. In hindsight I
> should have spotted that but didn't.
I think what I mean is that jump labels should always work. There
shouldn't be a need to #ifdef the common usercopy code. i.e.
include/linux/jump_label.h should work on all architectures already. I
see HAVE_JUMP_LABEL tests there, for example:
#if defined(CC_HAVE_ASM_GOTO) && defined(CONFIG_JUMP_LABEL)
# define HAVE_JUMP_LABEL
#endif
Other core code uses static keys without this; what is the failing combination?
-Kees
--
Kees Cook
Pixel Security
On 07/02/2018 04:54 PM, Kees Cook wrote:
> On Mon, Jul 2, 2018 at 11:55 AM, Christoph von Recklinghausen
> <[email protected]> wrote:
>> On 07/02/2018 02:43 PM, Kees Cook wrote:
>>> On Sat, Jun 30, 2018 at 1:43 PM, Christoph von Recklinghausen
>>> <[email protected]> wrote:
>>>> The last issue I'm chasing is build failures on ARCH=m68k. The error is
>>>> atomic_read and friends needed by the jump label code not being found.
>>>> The config has CONFIG_BROKEN_ON_SMP=y, so the jump label calls I added
>>>> will only be made #ifndef CONFIG_BROKEN_ON_SMP. Do you think that's
>>>> worth a mention in the blurb that's added to
>>>> Documentation/admin-guide/kernel-parameters.txt?
>>> Uhm, that's weird -- I think the configs on m68k need fixing then? I
>>> don't want to have to sprinkle that ifdef in generic code.
>>>
>>> How are other users of static keys and jump labels dealing with m68k weirdness?
>>>
>> There's also CONFIG_JUMP_LABEL which is defined in x86_64 but not
>> defined in the m68k configs. I'll use that instead. In hindsight I
>> should have spotted that but didn't.
> I think what I mean is that jump labels should always work. There
> shouldn't be a need to #ifdef the common usercopy code. i.e.
> include/linux/jump_label.h should work on all architectures already. I
> see HAVE_JUMP_LABEL tests there, for example:
>
> #if defined(CC_HAVE_ASM_GOTO) && defined(CONFIG_JUMP_LABEL)
> # define HAVE_JUMP_LABEL
> #endif
>
> Other core code uses static keys without this; what is the failing combination?
The complaints were when there was jump_label code in
include/linux/thread_info.h. Now that the code is isolated to
mm/usercopy.c, it successfully builds for m68k with mention of
CONFIG_JUMP_LABEL and CONFIG_SMP_BROKEN removed.
I'll send out a new patch in the morning after I test some more.
Thanks,
Chris
> -Kees
>
Hi Kees,
On Mon, Jul 2, 2018 at 8:44 PM Kees Cook <[email protected]> wrote:
> On Sat, Jun 30, 2018 at 1:43 PM, Christoph von Recklinghausen
> <[email protected]> wrote:
> > The last issue I'm chasing is build failures on ARCH=m68k. The error is
> > atomic_read and friends needed by the jump label code not being found.
> > The config has CONFIG_BROKEN_ON_SMP=y, so the jump label calls I added
> > will only be made #ifndef CONFIG_BROKEN_ON_SMP. Do you think that's
> > worth a mention in the blurb that's added to
> > Documentation/admin-guide/kernel-parameters.txt?
>
> Uhm, that's weird -- I think the configs on m68k need fixing then? I
> don't want to have to sprinkle that ifdef in generic code.
config BROKEN_ON_SMP
bool
depends on BROKEN || !SMP
default y
So BROKEN_ON_SMP=y if SMP=n, i.e. not an issue?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds