mbox series

[ovs-dev,0/3] gcc-plugins: Introduce stackinit plugin

Message ID 20190123110349.35882-1-keescook@chromium.org
Headers show
Series gcc-plugins: Introduce stackinit plugin | expand

Message

Kees Cook Jan. 23, 2019, 11:03 a.m. UTC 
This adds a new plugin "stackinit" that attempts to perform unconditional
initialization of all stack variables[1]. It has wider effects than
GCC_PLUGIN_STRUCTLEAK_BYREF_ALL=y since BYREF_ALL does not consider
non-structures. A notable weakness is that padding bytes in many cases
remain uninitialized since GCC treats these bytes as "undefined". I'm
hoping we can improve the compiler (or the plugin) to cover that too.
(It's worth noting that BYREF_ALL actually does handle the padding --
I think this is due to the different method of detecting if initialization
is needed.)

Included is a tree-wide change to move switch variables up and out of
their switch and into the top-level variable declarations.

Included is a set of test cases for evaluating stack initialization,
which checks for padding, different types, etc.

Feedback welcome! :)

-Kees

[1] https://lkml.kernel.org/r/CA+55aFykZL+cSBJjBBts7ebEFfyGPdMzTmLSxKnT_29=j942dA@mail.gmail.com

Kees Cook (3):
  treewide: Lift switch variables out of switches
  gcc-plugins: Introduce stackinit plugin
  lib: Introduce test_stackinit module

 arch/x86/xen/enlighten_pv.c                   |   7 +-
 drivers/char/pcmcia/cm4000_cs.c               |   2 +-
 drivers/char/ppdev.c                          |  20 +-
 drivers/gpu/drm/drm_edid.c                    |   4 +-
 drivers/gpu/drm/i915/intel_display.c          |   2 +-
 drivers/gpu/drm/i915/intel_pm.c               |   4 +-
 drivers/net/ethernet/intel/e1000/e1000_main.c |   3 +-
 drivers/tty/n_tty.c                           |   3 +-
 drivers/usb/gadget/udc/net2280.c              |   5 +-
 fs/fcntl.c                                    |   3 +-
 lib/Kconfig.debug                             |   9 +
 lib/Makefile                                  |   1 +
 lib/test_stackinit.c                          | 327 ++++++++++++++++++
 mm/shmem.c                                    |   5 +-
 net/core/skbuff.c                             |   4 +-
 net/ipv6/ip6_gre.c                            |   4 +-
 net/ipv6/ip6_tunnel.c                         |   4 +-
 net/openvswitch/flow_netlink.c                |   7 +-
 scripts/Makefile.gcc-plugins                  |   6 +
 scripts/gcc-plugins/Kconfig                   |   9 +
 scripts/gcc-plugins/gcc-common.h              |  11 +-
 scripts/gcc-plugins/stackinit_plugin.c        |  79 +++++
 security/tomoyo/common.c                      |   3 +-
 security/tomoyo/condition.c                   |   7 +-
 security/tomoyo/util.c                        |   4 +-
 25 files changed, 484 insertions(+), 49 deletions(-)
 create mode 100644 lib/test_stackinit.c
 create mode 100644 scripts/gcc-plugins/stackinit_plugin.c

Comments

Alexander Popov Jan. 29, 2019, 12:12 a.m. UTC | #1 
On 23.01.2019 14:03, Kees Cook wrote:
> This adds a new plugin "stackinit" that attempts to perform unconditional
> initialization of all stack variables

Hello Kees! Hello everyone!

I was curious about the performance impact of the initialization of all stack
variables. So I did a very brief test with this plugin on top of 4.20.5.

hackbench on Intel Core i7-4770 showed ~0.7% slowdown.
hackbench on Kirin 620 (ARM Cortex-A53 Octa-core 1.2GHz) showed ~1.3% slowdown.

This test involves the kernel scheduler and allocator. I can't say whether they
use stack aggressively. Maybe performance tests of other subsystems (e.g.
network subsystem) can show different numbers. Did you try?

I've heard a hypothesis that the initialization of all stack variables would
pollute CPU caches, which is critical for some types of computations. Maybe some
micro-benchmarks can disprove/confirm that?

Thanks!
Best regards,
Alexander
Kees Cook Feb. 12, 2019, 5:54 p.m. UTC | #2 
On Mon, Jan 28, 2019 at 4:12 PM Alexander Popov <alex.popov@linux.com> wrote:
>
> On 23.01.2019 14:03, Kees Cook wrote:
> > This adds a new plugin "stackinit" that attempts to perform unconditional
> > initialization of all stack variables
>
> Hello Kees! Hello everyone!
>
> I was curious about the performance impact of the initialization of all stack
> variables. So I did a very brief test with this plugin on top of 4.20.5.
>
> hackbench on Intel Core i7-4770 showed ~0.7% slowdown.
> hackbench on Kirin 620 (ARM Cortex-A53 Octa-core 1.2GHz) showed ~1.3% slowdown.

Thanks for looking at this! I'll be including my hackbench
measurements for the v2 here in a moment.

> This test involves the kernel scheduler and allocator. I can't say whether they
> use stack aggressively. Maybe performance tests of other subsystems (e.g.
> network subsystem) can show different numbers. Did you try?

I haven't found a stable network test yet. If someone can find a
reasonable workload, I'd love to hear about it.

> I've heard a hypothesis that the initialization of all stack variables would
> pollute CPU caches, which is critical for some types of computations. Maybe some
> micro-benchmarks can disprove/confirm that?

I kind of think micro-benchmarks aren't so useful because they don't
represent a real-world workload. I've heard people talk about SAP-HANA
as a good test, but I can't get my hands on it. I wonder if anyone has
tried "mysqlslap"?