[v3] add param that allows bootline control of hardened usercopy
diff mbox series

Message ID 1530101255-13988-1-git-send-email-crecklin@redhat.com
State New, archived
Headers show
Series
  • [v3] add param that allows bootline control of hardened usercopy
Related show

Commit Message

Chris von Recklinghausen June 27, 2018, 12:07 p.m. UTC
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 <crecklin@redhat.com>
---
 .../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(+)

Comments

kernel test robot June 30, 2018, 3:26 a.m. UTC | #1
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 <pbonzini@redhat.com>
:::::: CC: Ingo Molnar <mingo@kernel.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
kernel test robot June 30, 2018, 6:40 a.m. UTC | #2
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 <pbonzini@redhat.com>
:::::: CC: Ingo Molnar <mingo@kernel.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Kees Cook June 30, 2018, 8:11 p.m. UTC | #3
On Wed, Jun 27, 2018 at 5:07 AM, Chris von Recklinghausen
<crecklin@redhat.com> 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 <crecklin@redhat.com>
> ---
>  .../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
Chris von Recklinghausen June 30, 2018, 8:43 p.m. UTC | #4
On 06/30/2018 04:11 PM, Kees Cook wrote:
> On Wed, Jun 27, 2018 at 5:07 AM, Chris von Recklinghausen
> <crecklin@redhat.com> 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 <crecklin@redhat.com>
>> ---
>>  .../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
>
Kees Cook July 2, 2018, 6:43 p.m. UTC | #5
On Sat, Jun 30, 2018 at 1:43 PM, Christoph von Recklinghausen
<crecklin@redhat.com> 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
Chris von Recklinghausen July 2, 2018, 6:55 p.m. UTC | #6
On 07/02/2018 02:43 PM, Kees Cook wrote:
> On Sat, Jun 30, 2018 at 1:43 PM, Christoph von Recklinghausen
> <crecklin@redhat.com> 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
Kees Cook July 2, 2018, 8:54 p.m. UTC | #7
On Mon, Jul 2, 2018 at 11:55 AM, Christoph von Recklinghausen
<crecklin@redhat.com> wrote:
> On 07/02/2018 02:43 PM, Kees Cook wrote:
>> On Sat, Jun 30, 2018 at 1:43 PM, Christoph von Recklinghausen
>> <crecklin@redhat.com> 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
Chris von Recklinghausen July 2, 2018, 10:23 p.m. UTC | #8
On 07/02/2018 04:54 PM, Kees Cook wrote:
> On Mon, Jul 2, 2018 at 11:55 AM, Christoph von Recklinghausen
> <crecklin@redhat.com> wrote:
>> On 07/02/2018 02:43 PM, Kees Cook wrote:
>>> On Sat, Jun 30, 2018 at 1:43 PM, Christoph von Recklinghausen
>>> <crecklin@redhat.com> 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
>
Geert Uytterhoeven July 3, 2018, 8:04 a.m. UTC | #9
Hi Kees,

On Mon, Jul 2, 2018 at 8:44 PM Kees Cook <keescook@chromium.org> wrote:
> On Sat, Jun 30, 2018 at 1:43 PM, Christoph von Recklinghausen
> <crecklin@redhat.com> 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

Patch
diff mbox series

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);