| Message ID | 20201029183526.2131776-2-aleksandrnogikh@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | security: add fault injection to LSM hooks | expand |
On Thu, 29 Oct 2020 at 19:35, Aleksandr Nogikh <aleksandrnogikh@gmail.com> wrote: > From: Aleksandr Nogikh <nogikh@google.com> > > Add a fault injection capability to call_int_hook macro. This will > facilitate testing of fault tolerance of the code that invokes > security hooks as well as the fault tolerance of the LSM > implementations themselves. > > Add a KConfig option (CONFIG_FAIL_LSM_HOOKS) that controls whether the > capability is enabled. In order to enable configuration from the user > space, add the standard debugfs entries for fault injection (if > CONFIG_FAULT_INJECTION_DEBUG_FS is enabled). > > Signed-off-by: Aleksandr Nogikh <nogikh@google.com> Reviewed-by: Marco Elver <elver@google.com> > --- > v2: > * Renamed should_fail_lsm_hook() to lsm_hooks_inject_fail(). > --- > lib/Kconfig.debug | 6 +++++ > security/security.c | 53 ++++++++++++++++++++++++++++++++++++++++++--- > 2 files changed, 56 insertions(+), 3 deletions(-) > > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug > index 537cf3c2937d..80d289591e29 100644 > --- a/lib/Kconfig.debug > +++ b/lib/Kconfig.debug > @@ -1803,6 +1803,12 @@ config FAIL_MAKE_REQUEST > help > Provide fault-injection capability for disk IO. > > +config FAIL_LSM_HOOKS > + bool "Fault-injection capability for LSM hooks" > + depends on FAULT_INJECTION > + help > + Provide fault-injection capability for LSM hooks. > + > config FAIL_IO_TIMEOUT > bool "Fault-injection capability for faking disk interrupts" > depends on FAULT_INJECTION && BLOCK > diff --git a/security/security.c b/security/security.c > index 69ff6e2e2cd4..1105ad0f6891 100644 > --- a/security/security.c > +++ b/security/security.c > @@ -28,6 +28,7 @@ > #include <linux/backing-dev.h> > #include <linux/string.h> > #include <linux/msg.h> > +#include <linux/fault-inject.h> > #include <net/flow.h> > > #define MAX_LSM_EVM_XATTR 2 > @@ -669,6 +670,51 @@ static void __init lsm_early_task(struct task_struct *task) > panic("%s: Early task alloc failed.\n", __func__); > } > > + > +#ifdef CONFIG_FAIL_LSM_HOOKS > + > +static struct { > + struct fault_attr attr; > + int retval; > +} fail_lsm_hooks = { > + .attr = FAULT_ATTR_INITIALIZER, > + .retval = -EACCES > +}; > + > +static int __init setup_fail_lsm_hooks(char *str) > +{ > + return setup_fault_attr(&fail_lsm_hooks.attr, str); > +} > +__setup("fail_lsm_hooks=", setup_fail_lsm_hooks); > + > +static int lsm_hooks_inject_fail(void) > +{ > + return should_fail(&fail_lsm_hooks.attr, 1) ? fail_lsm_hooks.retval : 0; > +} > + > +#ifdef CONFIG_FAULT_INJECTION_DEBUG_FS > + > +static int __init fail_lsm_hooks_debugfs(void) > +{ > + umode_t mode = S_IFREG | 0600; > + struct dentry *dir; > + > + dir = fault_create_debugfs_attr("fail_lsm_hooks", NULL, > + &fail_lsm_hooks.attr); > + debugfs_create_u32("retval", mode, dir, &fail_lsm_hooks.retval); > + return 0; > +} > + > +late_initcall(fail_lsm_hooks_debugfs); > + > +#endif /* CONFIG_FAULT_INJECTION_DEBUG_FS */ > + > +#else > + > +static inline int lsm_hooks_inject_fail(void) { return 0; } > + > +#endif /* CONFIG_FAIL_LSM_HOOKS */ > + > /* > * The default value of the LSM hook is defined in linux/lsm_hook_defs.h and > * can be accessed with: > @@ -707,16 +753,17 @@ static void __init lsm_early_task(struct task_struct *task) > } while (0) > > #define call_int_hook(FUNC, IRC, ...) ({ \ > - int RC = IRC; \ > - do { \ > + int RC = lsm_hooks_inject_fail(); \ > + if (RC == 0) { \ > struct security_hook_list *P; \ > + RC = IRC; \ > \ > hlist_for_each_entry(P, &security_hook_heads.FUNC, list) { \ > RC = P->hook.FUNC(__VA_ARGS__); \ > if (RC != 0) \ > break; \ > } \ > - } while (0); \ > + } \ > RC; \ > }) > > -- > 2.29.1.341.ge80a0c044ae-goog >
On 2020/10/30 3:35, Aleksandr Nogikh wrote: > +#ifdef CONFIG_FAIL_LSM_HOOKS > + > +static struct { > + struct fault_attr attr; > + int retval; > +} fail_lsm_hooks = { > + .attr = FAULT_ATTR_INITIALIZER, > + .retval = -EACCES > +}; > + > +static int __init setup_fail_lsm_hooks(char *str) > +{ > + return setup_fault_attr(&fail_lsm_hooks.attr, str); > +} > +__setup("fail_lsm_hooks=", setup_fail_lsm_hooks); > + > +static int lsm_hooks_inject_fail(void) > +{ > + return should_fail(&fail_lsm_hooks.attr, 1) ? fail_lsm_hooks.retval : 0; > +} > + > +#ifdef CONFIG_FAULT_INJECTION_DEBUG_FS > + > +static int __init fail_lsm_hooks_debugfs(void) > +{ > + umode_t mode = S_IFREG | 0600; > + struct dentry *dir; > + > + dir = fault_create_debugfs_attr("fail_lsm_hooks", NULL, > + &fail_lsm_hooks.attr); > + debugfs_create_u32("retval", mode, dir, &fail_lsm_hooks.retval); Since production kernels will use CONFIG_FAIL_LSM_HOOKS=n, we won't need to worry about userspace ABI. Reviewed-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> By the way, fail_lsm_hooks.retval is "signed int" but debugfs_create_u32() handles "unsigned int". Do we want to allow lsm_hooks_inject_fail() to inject arbitrary !IS_ERR_VALUE() values? > + return 0; > +}
On Thu, Oct 29, 2020 at 7:35 PM Aleksandr Nogikh <aleksandrnogikh@gmail.com> wrote: > > From: Aleksandr Nogikh <nogikh@google.com> > > Add a fault injection capability to call_int_hook macro. This will > facilitate testing of fault tolerance of the code that invokes > security hooks as well as the fault tolerance of the LSM > implementations themselves. > > Add a KConfig option (CONFIG_FAIL_LSM_HOOKS) that controls whether the > capability is enabled. In order to enable configuration from the user > space, add the standard debugfs entries for fault injection (if > CONFIG_FAULT_INJECTION_DEBUG_FS is enabled). > > Signed-off-by: Aleksandr Nogikh <nogikh@google.com> Reviewed-by: Andrey Konovalov <andreyknvl@google.com> > --- > v2: > * Renamed should_fail_lsm_hook() to lsm_hooks_inject_fail(). > --- > lib/Kconfig.debug | 6 +++++ > security/security.c | 53 ++++++++++++++++++++++++++++++++++++++++++--- > 2 files changed, 56 insertions(+), 3 deletions(-) > > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug > index 537cf3c2937d..80d289591e29 100644 > --- a/lib/Kconfig.debug > +++ b/lib/Kconfig.debug > @@ -1803,6 +1803,12 @@ config FAIL_MAKE_REQUEST > help > Provide fault-injection capability for disk IO. > > +config FAIL_LSM_HOOKS > + bool "Fault-injection capability for LSM hooks" > + depends on FAULT_INJECTION > + help > + Provide fault-injection capability for LSM hooks. > + > config FAIL_IO_TIMEOUT > bool "Fault-injection capability for faking disk interrupts" > depends on FAULT_INJECTION && BLOCK > diff --git a/security/security.c b/security/security.c > index 69ff6e2e2cd4..1105ad0f6891 100644 > --- a/security/security.c > +++ b/security/security.c > @@ -28,6 +28,7 @@ > #include <linux/backing-dev.h> > #include <linux/string.h> > #include <linux/msg.h> > +#include <linux/fault-inject.h> > #include <net/flow.h> > > #define MAX_LSM_EVM_XATTR 2 > @@ -669,6 +670,51 @@ static void __init lsm_early_task(struct task_struct *task) > panic("%s: Early task alloc failed.\n", __func__); > } > > + > +#ifdef CONFIG_FAIL_LSM_HOOKS > + > +static struct { > + struct fault_attr attr; > + int retval; > +} fail_lsm_hooks = { > + .attr = FAULT_ATTR_INITIALIZER, > + .retval = -EACCES > +}; > + > +static int __init setup_fail_lsm_hooks(char *str) > +{ > + return setup_fault_attr(&fail_lsm_hooks.attr, str); > +} > +__setup("fail_lsm_hooks=", setup_fail_lsm_hooks); > + > +static int lsm_hooks_inject_fail(void) > +{ > + return should_fail(&fail_lsm_hooks.attr, 1) ? fail_lsm_hooks.retval : 0; > +} > + > +#ifdef CONFIG_FAULT_INJECTION_DEBUG_FS > + > +static int __init fail_lsm_hooks_debugfs(void) > +{ > + umode_t mode = S_IFREG | 0600; > + struct dentry *dir; > + > + dir = fault_create_debugfs_attr("fail_lsm_hooks", NULL, > + &fail_lsm_hooks.attr); > + debugfs_create_u32("retval", mode, dir, &fail_lsm_hooks.retval); > + return 0; > +} > + > +late_initcall(fail_lsm_hooks_debugfs); > + > +#endif /* CONFIG_FAULT_INJECTION_DEBUG_FS */ > + > +#else > + > +static inline int lsm_hooks_inject_fail(void) { return 0; } > + > +#endif /* CONFIG_FAIL_LSM_HOOKS */ > + > /* > * The default value of the LSM hook is defined in linux/lsm_hook_defs.h and > * can be accessed with: > @@ -707,16 +753,17 @@ static void __init lsm_early_task(struct task_struct *task) > } while (0) > > #define call_int_hook(FUNC, IRC, ...) ({ \ > - int RC = IRC; \ > - do { \ > + int RC = lsm_hooks_inject_fail(); \ > + if (RC == 0) { \ > struct security_hook_list *P; \ > + RC = IRC; \ > \ > hlist_for_each_entry(P, &security_hook_heads.FUNC, list) { \ > RC = P->hook.FUNC(__VA_ARGS__); \ > if (RC != 0) \ > break; \ > } \ > - } while (0); \ > + } \ > RC; \ > }) > > -- > 2.29.1.341.ge80a0c044ae-goog >
On Tue, Nov 10, 2020 at 7:43 AM Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp> wrote: > [...] > > By the way, fail_lsm_hooks.retval is "signed int" but debugfs_create_u32() handles "unsigned int". > Do we want to allow lsm_hooks_inject_fail() to inject arbitrary !IS_ERR_VALUE() values? Thanks for pointing it out. Technically, now it's possible to set a negative value - internally, the kernel will process negative integers anyway, and after casting the unsigned value to a signed one, retval will contain exactly what the user provided. However, if the user retrieves the attribute value, they won't get the exact value that was set (if it was negative). I'll change debugfs_create_u32 to something else in v4, so that it'll be more explicit and so that it'll be possible to read negative values normally.
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index 537cf3c2937d..80d289591e29 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -1803,6 +1803,12 @@ config FAIL_MAKE_REQUEST help Provide fault-injection capability for disk IO. +config FAIL_LSM_HOOKS + bool "Fault-injection capability for LSM hooks" + depends on FAULT_INJECTION + help + Provide fault-injection capability for LSM hooks. + config FAIL_IO_TIMEOUT bool "Fault-injection capability for faking disk interrupts" depends on FAULT_INJECTION && BLOCK diff --git a/security/security.c b/security/security.c index 69ff6e2e2cd4..1105ad0f6891 100644 --- a/security/security.c +++ b/security/security.c @@ -28,6 +28,7 @@ #include <linux/backing-dev.h> #include <linux/string.h> #include <linux/msg.h> +#include <linux/fault-inject.h> #include <net/flow.h> #define MAX_LSM_EVM_XATTR 2 @@ -669,6 +670,51 @@ static void __init lsm_early_task(struct task_struct *task) panic("%s: Early task alloc failed.\n", __func__); } + +#ifdef CONFIG_FAIL_LSM_HOOKS + +static struct { + struct fault_attr attr; + int retval; +} fail_lsm_hooks = { + .attr = FAULT_ATTR_INITIALIZER, + .retval = -EACCES +}; + +static int __init setup_fail_lsm_hooks(char *str) +{ + return setup_fault_attr(&fail_lsm_hooks.attr, str); +} +__setup("fail_lsm_hooks=", setup_fail_lsm_hooks); + +static int lsm_hooks_inject_fail(void) +{ + return should_fail(&fail_lsm_hooks.attr, 1) ? fail_lsm_hooks.retval : 0; +} + +#ifdef CONFIG_FAULT_INJECTION_DEBUG_FS + +static int __init fail_lsm_hooks_debugfs(void) +{ + umode_t mode = S_IFREG | 0600; + struct dentry *dir; + + dir = fault_create_debugfs_attr("fail_lsm_hooks", NULL, + &fail_lsm_hooks.attr); + debugfs_create_u32("retval", mode, dir, &fail_lsm_hooks.retval); + return 0; +} + +late_initcall(fail_lsm_hooks_debugfs); + +#endif /* CONFIG_FAULT_INJECTION_DEBUG_FS */ + +#else + +static inline int lsm_hooks_inject_fail(void) { return 0; } + +#endif /* CONFIG_FAIL_LSM_HOOKS */ + /* * The default value of the LSM hook is defined in linux/lsm_hook_defs.h and * can be accessed with: @@ -707,16 +753,17 @@ static void __init lsm_early_task(struct task_struct *task) } while (0) #define call_int_hook(FUNC, IRC, ...) ({ \ - int RC = IRC; \ - do { \ + int RC = lsm_hooks_inject_fail(); \ + if (RC == 0) { \ struct security_hook_list *P; \ + RC = IRC; \ \ hlist_for_each_entry(P, &security_hook_heads.FUNC, list) { \ RC = P->hook.FUNC(__VA_ARGS__); \ if (RC != 0) \ break; \ } \ - } while (0); \ + } \ RC; \ })