Message-Id: <20201026125227.54520-1-a.nogikh@gmail.com>
Date:   Mon, 26 Oct 2020 12:52:25 +0000
From:   Aleksandr Nogikh <a.nogikh@...il.com>
To:     jmorris@...ei.org, serge@...lyn.com, akinobu.mita@...il.com
Cc:     andreyknvl@...gle.com, dvyukov@...gle.com, elver@...gle.com,
        glider@...gle.com, keescook@...gle.com,
        linux-kernel@...r.kernel.org,
        linux-security-module@...r.kernel.org,
        Aleksandr Nogikh <nogikh@...gle.com>
Subject: [RFC PATCH v2 0/2] security: add fault injection to LSM hooks

From: Aleksandr Nogikh <nogikh@...gle.com>

Fault injection capabilities[Documentation/fault-injection/fault-injection.rst]
facilitate testing of the stability of the Linux kernel by providing
means to force a number of kernel interfaces to return error
codes. This RFC proposes adding such fault injection capability into
LSM hooks.

The intent is to make it possible to test whether the existing kernel
code properly handles negative return values of LSM hooks. Syzbot
[https://github.com/google/syzkaller/blob/master/docs/syzbot.md] will
automatically do that with the aid of instrumentation tools once these
changes are merged.

Is the attached implementation consistent with the ideas behind LSM
stacking in its current state? In particular, is it reasonable to
expect the existing LSMs to operate normally when they are active and
such fault injection happens?

Local fuzzing of a Linux kernel with this patch has almost instantly
led to two crashes. I'm not sure whether they correspond to actual
issues as the LSM fault injection implementation (and the concept
itself) can be wrong. Here they are:

1. "general protection fault in selinux_inode_free_security". This is
caused by executing security_inode_free() when a fault was injected to
inode_alloc_security() and therefore selinux_inode_alloc_security()
was not executed. In this case, the subsequent inode_free_security()
call executes list_del_init() on an uninitialized list. Theoretically,
this may happen if some other LSM precedes selinux in the hooks list
and its inode_alloc_security hook fails.

A fault was injected to this call_int_hook():
https://elixir.bootlin.com/linux/v5.9/source/security/security.c#L975

Below you can find a call trace for the subsequent crash.
__list_del_entry include/linux/list.h:132 [inline]
list_del_init include/linux/list.h:204 [inline]
inode_free_security security/selinux/hooks.c:337 [inline]
selinux_inode_free_security+0xf0/0x290 security/selinux/hooks.c:2839
security_inode_free+0x46/0xc0 security/security.c:1042
security_inode_alloc+0x161/0x1a0 security/security.c:1027
inode_init_always+0x5a7/0xd10 fs/inode.c:171
alloc_inode+0x82/0x230 fs/inode.c:239
new_inode_pseudo+0x14/0xe0 fs/inode.c:928
sock_alloc+0x3c/0x260 net/socket.c:573
__sock_create+0xb9/0x780 net/socket.c:1391
sock_create net/socket.c:1478 [inline]
__sys_socket+0xef/0x200 net/socket.c:1520
__do_sys_socket net/socket.c:1529 [inline]
__se_sys_socket net/socket.c:1527 [inline]
__x64_sys_socket+0x6f/0xb0 net/socket.c:1527
do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46
entry_SYSCALL_64_after_hwframe+0x44/0xa9

2. BUG_ON inside security_skb_classify_flow(). Why is it needed there?
https://elixir.bootlin.com/linux/v5.9/source/security/security.c#L2426

---
v2:
* Renamed should_fail_lsm_hook() to should_fail_lsm_hook().
* Extended the documentation.

Aleksandr Nogikh (2):
  security: add fault injection capability
  docs: add fail_lsm_hooks info to fault-injection.rst

 .../fault-injection/fault-injection.rst       |  6 +++
 lib/Kconfig.debug                             |  6 +++
 security/security.c                           | 53 +++++++++++++++++--
 3 files changed, 62 insertions(+), 3 deletions(-)


base-commit: 2ef991b5fdbe828dc8fb8af473dab160729570ed
-- 
2.29.0.rc1.297.gfa9743e501-goog

Powered by blists - more mailing lists