2020-08-28 14:16:22

by albert.linde

[permalink] [raw]
Subject: [PATCH v2 0/3] add fault injection to user memory access

From: Albert van der Linde <[email protected]>

The goal of this series is to improve testing of fault-tolerance in
usages of user memory access functions, by adding support for fault
injection.

The first patch adds failure injection capability for usercopy
functions. The second changes usercopy functions to use this new failure
capability (copy_from_user, ...). The third patch adds
get/put/clear_user failures to x86.

Changes in v2:
- simplified overall failure capability by either failing or not, without
having functions fail partially by copying/clearing only some bytes

Albert van der Linde (3):
lib, include/linux: add usercopy failure capability
lib, uaccess: add failure injection to usercopy functions
x86: add failure injection to get/put/clear_user

.../admin-guide/kernel-parameters.txt | 1 +
.../fault-injection/fault-injection.rst | 7 +-
arch/x86/include/asm/uaccess.h | 68 +++++++++++--------
arch/x86/lib/usercopy_64.c | 3 +
include/linux/fault-inject-usercopy.h | 22 ++++++
include/linux/uaccess.h | 11 ++-
lib/Kconfig.debug | 7 ++
lib/Makefile | 1 +
lib/fault-inject-usercopy.c | 39 +++++++++++
lib/iov_iter.c | 5 ++
lib/strncpy_from_user.c | 3 +
lib/usercopy.c | 5 +-
12 files changed, 140 insertions(+), 32 deletions(-)
create mode 100644 include/linux/fault-inject-usercopy.h
create mode 100644 lib/fault-inject-usercopy.c

--
2.28.0.402.g5ffc5be6b7-goog


2020-08-28 14:16:40

by albert.linde

[permalink] [raw]
Subject: [PATCH v2 1/3] lib, include/linux: add usercopy failure capability

From: Albert van der Linde <[email protected]>

Add a failure injection capability to improve testing of fault-tolerance
in usages of user memory access functions.

Add CONFIG_FAULT_INJECTION_USERCOPY to enable faults in usercopy
functions. The should_fail_usercopy function is to be called by these
functions (copy_from_user, get_user, ...) in order to fail or not.

Signed-off-by: Albert van der Linde <[email protected]>
---
v2:
- adressed comments from Dmitry Vyukov
- removed failsize
- changed should_fail function to return bool
---
.../admin-guide/kernel-parameters.txt | 1 +
.../fault-injection/fault-injection.rst | 7 +++-
include/linux/fault-inject-usercopy.h | 22 +++++++++++
lib/Kconfig.debug | 7 ++++
lib/Makefile | 1 +
lib/fault-inject-usercopy.c | 39 +++++++++++++++++++
6 files changed, 76 insertions(+), 1 deletion(-)
create mode 100644 include/linux/fault-inject-usercopy.h
create mode 100644 lib/fault-inject-usercopy.c

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index a1068742a6df..790e54988d4f 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1332,6 +1332,7 @@
current integrity status.

failslab=
+ fail_usercopy=
fail_page_alloc=
fail_make_request=[KNL]
General fault injection mechanism.
diff --git a/Documentation/fault-injection/fault-injection.rst b/Documentation/fault-injection/fault-injection.rst
index f850ad018b70..31ecfe44e5b4 100644
--- a/Documentation/fault-injection/fault-injection.rst
+++ b/Documentation/fault-injection/fault-injection.rst
@@ -16,6 +16,10 @@ Available fault injection capabilities

injects page allocation failures. (alloc_pages(), get_free_pages(), ...)

+- fail_usercopy
+
+ injects failures in user memory access functions. (copy_from_user(), get_user(), ...)
+
- fail_futex

injects futex deadlock and uaddr fault errors.
@@ -177,6 +181,7 @@ use the boot option::

failslab=
fail_page_alloc=
+ fail_usercopy=
fail_make_request=
fail_futex=
mmc_core.fail_request=<interval>,<probability>,<space>,<times>
@@ -222,7 +227,7 @@ How to add new fault injection capability

- debugfs entries

- failslab, fail_page_alloc, and fail_make_request use this way.
+ failslab, fail_page_alloc, fail_usercopy, and fail_make_request use this way.
Helper functions:

fault_create_debugfs_attr(name, parent, attr);
diff --git a/include/linux/fault-inject-usercopy.h b/include/linux/fault-inject-usercopy.h
new file mode 100644
index 000000000000..56c3a693fdd9
--- /dev/null
+++ b/include/linux/fault-inject-usercopy.h
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __LINUX_FAULT_INJECT_USERCOPY_H__
+#define __LINUX_FAULT_INJECT_USERCOPY_H__
+
+/*
+ * This header provides a wrapper for injecting failures to user space memory
+ * access functions.
+ */
+
+#include <linux/types.h>
+
+#ifdef CONFIG_FAULT_INJECTION_USERCOPY
+
+bool should_fail_usercopy(void);
+
+#else
+
+static inline bool should_fail_usercopy(void) { return false; }
+
+#endif /* CONFIG_FAULT_INJECTION_USERCOPY */
+
+#endif /* __LINUX_FAULT_INJECT_USERCOPY_H__ */
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index e068c3c7189a..2fc5049fba4e 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1770,6 +1770,13 @@ config FAIL_PAGE_ALLOC
help
Provide fault-injection capability for alloc_pages().

+config FAULT_INJECTION_USERCOPY
+ bool "Fault injection capability for usercopy functions"
+ depends on FAULT_INJECTION
+ help
+ Provides fault-injection capability to inject failures
+ in usercopy functions (copy_from_user(), get_user(), ...).
+
config FAIL_MAKE_REQUEST
bool "Fault-injection capability for disk IO"
depends on FAULT_INJECTION && BLOCK
diff --git a/lib/Makefile b/lib/Makefile
index a4a4c6864f51..18daad2bc606 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -207,6 +207,7 @@ obj-$(CONFIG_AUDIT_COMPAT_GENERIC) += compat_audit.o

obj-$(CONFIG_IOMMU_HELPER) += iommu-helper.o
obj-$(CONFIG_FAULT_INJECTION) += fault-inject.o
+obj-$(CONFIG_FAULT_INJECTION_USERCOPY) += fault-inject-usercopy.o
obj-$(CONFIG_NOTIFIER_ERROR_INJECTION) += notifier-error-inject.o
obj-$(CONFIG_PM_NOTIFIER_ERROR_INJECT) += pm-notifier-error-inject.o
obj-$(CONFIG_NETDEV_NOTIFIER_ERROR_INJECT) += netdev-notifier-error-inject.o
diff --git a/lib/fault-inject-usercopy.c b/lib/fault-inject-usercopy.c
new file mode 100644
index 000000000000..77558b6c29ca
--- /dev/null
+++ b/lib/fault-inject-usercopy.c
@@ -0,0 +1,39 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <linux/fault-inject.h>
+#include <linux/fault-inject-usercopy.h>
+
+static struct {
+ struct fault_attr attr;
+} fail_usercopy = {
+ .attr = FAULT_ATTR_INITIALIZER,
+};
+
+static int __init setup_fail_usercopy(char *str)
+{
+ return setup_fault_attr(&fail_usercopy.attr, str);
+}
+__setup("fail_usercopy=", setup_fail_usercopy);
+
+#ifdef CONFIG_FAULT_INJECTION_DEBUG_FS
+
+static int __init fail_usercopy_debugfs(void)
+{
+ struct dentry *dir;
+
+ dir = fault_create_debugfs_attr("fail_usercopy", NULL,
+ &fail_usercopy.attr);
+ if (IS_ERR(dir))
+ return PTR_ERR(dir);
+
+ return 0;
+}
+
+late_initcall(fail_usercopy_debugfs);
+
+#endif /* CONFIG_FAULT_INJECTION_DEBUG_FS */
+
+bool should_fail_usercopy(void)
+{
+ return should_fail(&fail_usercopy.attr, 1);
+}
+EXPORT_SYMBOL_GPL(should_fail_usercopy);
--
2.28.0.402.g5ffc5be6b7-goog

2020-08-28 14:17:05

by albert.linde

[permalink] [raw]
Subject: [PATCH v2 2/3] lib, uaccess: add failure injection to usercopy functions

From: Albert van der Linde <[email protected]>

To test fault-tolerance of user memory access functions, introduce fault
injection to usercopy functions.

If a failure is expected return either -EFAULT or the total amount of
bytes that were not copied.

Signed-off-by: Albert van der Linde <[email protected]>
---
v2:
- removed partial failures
---
include/linux/uaccess.h | 11 ++++++++++-
lib/iov_iter.c | 5 +++++
lib/strncpy_from_user.c | 3 +++
lib/usercopy.c | 5 ++++-
4 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h
index 94b285411659..7fd54596af17 100644
--- a/include/linux/uaccess.h
+++ b/include/linux/uaccess.h
@@ -2,6 +2,7 @@
#ifndef __LINUX_UACCESS_H__
#define __LINUX_UACCESS_H__

+#include <linux/fault-inject-usercopy.h>
#include <linux/instrumented.h>
#include <linux/sched.h>
#include <linux/thread_info.h>
@@ -82,6 +83,8 @@ __copy_from_user_inatomic(void *to, const void __user *from, unsigned long n)
static __always_inline __must_check unsigned long
__copy_from_user(void *to, const void __user *from, unsigned long n)
{
+ if (should_fail_usercopy())
+ return n;
might_fault();
instrument_copy_from_user(to, from, n);
check_object_size(to, n, false);
@@ -104,6 +107,8 @@ __copy_from_user(void *to, const void __user *from, unsigned long n)
static __always_inline __must_check unsigned long
__copy_to_user_inatomic(void __user *to, const void *from, unsigned long n)
{
+ if (should_fail_usercopy())
+ return n;
instrument_copy_to_user(to, from, n);
check_object_size(from, n, true);
return raw_copy_to_user(to, from, n);
@@ -112,6 +117,8 @@ __copy_to_user_inatomic(void __user *to, const void *from, unsigned long n)
static __always_inline __must_check unsigned long
__copy_to_user(void __user *to, const void *from, unsigned long n)
{
+ if (should_fail_usercopy())
+ return n;
might_fault();
instrument_copy_to_user(to, from, n);
check_object_size(from, n, true);
@@ -124,7 +131,7 @@ _copy_from_user(void *to, const void __user *from, unsigned long n)
{
unsigned long res = n;
might_fault();
- if (likely(access_ok(from, n))) {
+ if (!should_fail_usercopy() && likely(access_ok(from, n))) {
instrument_copy_from_user(to, from, n);
res = raw_copy_from_user(to, from, n);
}
@@ -141,6 +148,8 @@ _copy_from_user(void *, const void __user *, unsigned long);
static inline __must_check unsigned long
_copy_to_user(void __user *to, const void *from, unsigned long n)
{
+ if (should_fail_usercopy())
+ return n;
might_fault();
if (access_ok(to, n)) {
instrument_copy_to_user(to, from, n);
diff --git a/lib/iov_iter.c b/lib/iov_iter.c
index 5e40786c8f12..eeac08855b24 100644
--- a/lib/iov_iter.c
+++ b/lib/iov_iter.c
@@ -2,6 +2,7 @@
#include <crypto/hash.h>
#include <linux/export.h>
#include <linux/bvec.h>
+#include <linux/fault-inject-usercopy.h>
#include <linux/uio.h>
#include <linux/pagemap.h>
#include <linux/slab.h>
@@ -139,6 +140,8 @@

static int copyout(void __user *to, const void *from, size_t n)
{
+ if (should_fail_usercopy())
+ return n;
if (access_ok(to, n)) {
instrument_copy_to_user(to, from, n);
n = raw_copy_to_user(to, from, n);
@@ -148,6 +151,8 @@ static int copyout(void __user *to, const void *from, size_t n)

static int copyin(void *to, const void __user *from, size_t n)
{
+ if (should_fail_usercopy())
+ return n;
if (access_ok(from, n)) {
instrument_copy_from_user(to, from, n);
n = raw_copy_from_user(to, from, n);
diff --git a/lib/strncpy_from_user.c b/lib/strncpy_from_user.c
index 34696a348864..eec7065e6342 100644
--- a/lib/strncpy_from_user.c
+++ b/lib/strncpy_from_user.c
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: GPL-2.0
#include <linux/compiler.h>
#include <linux/export.h>
+#include <linux/fault-inject-usercopy.h>
#include <linux/kasan-checks.h>
#include <linux/thread_info.h>
#include <linux/uaccess.h>
@@ -98,6 +99,8 @@ long strncpy_from_user(char *dst, const char __user *src, long count)
{
unsigned long max_addr, src_addr;

+ if (should_fail_usercopy())
+ return -EFAULT;
might_fault();
if (unlikely(count <= 0))
return 0;
diff --git a/lib/usercopy.c b/lib/usercopy.c
index b26509f112f9..766b6f8a6937 100644
--- a/lib/usercopy.c
+++ b/lib/usercopy.c
@@ -1,5 +1,6 @@
// SPDX-License-Identifier: GPL-2.0
#include <linux/bitops.h>
+#include <linux/fault-inject-usercopy.h>
#include <linux/instrumented.h>
#include <linux/uaccess.h>

@@ -10,7 +11,7 @@ unsigned long _copy_from_user(void *to, const void __user *from, unsigned long n
{
unsigned long res = n;
might_fault();
- if (likely(access_ok(from, n))) {
+ if (!should_fail_usercopy() && likely(access_ok(from, n))) {
instrument_copy_from_user(to, from, n);
res = raw_copy_from_user(to, from, n);
}
@@ -24,6 +25,8 @@ EXPORT_SYMBOL(_copy_from_user);
#ifndef INLINE_COPY_TO_USER
unsigned long _copy_to_user(void __user *to, const void *from, unsigned long n)
{
+ if (should_fail_usercopy())
+ return n;
might_fault();
if (likely(access_ok(to, n))) {
instrument_copy_to_user(to, from, n);
--
2.28.0.402.g5ffc5be6b7-goog

2020-08-31 14:31:23

by Akinobu Mita

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] add fault injection to user memory access

2020年8月28日(金) 23:14 <[email protected]>:
>
> From: Albert van der Linde <[email protected]>
>
> The goal of this series is to improve testing of fault-tolerance in
> usages of user memory access functions, by adding support for fault
> injection.
>
> The first patch adds failure injection capability for usercopy
> functions. The second changes usercopy functions to use this new failure
> capability (copy_from_user, ...). The third patch adds
> get/put/clear_user failures to x86.

This series looks good to me.

Reviewed-by: Akinobu Mita <[email protected]>

2020-08-31 16:31:08

by Akinobu Mita

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] add fault injection to user memory access

Andrew,

Could you take a look at this series, and consider taking in -mm tree?

2020年9月1日(火) 0:49 Alexander Potapenko <[email protected]>:
>
> > This series looks good to me.
>
> Great!
>
> Which tree do fault injection patches normally go to?
>
> > Reviewed-by: Akinobu Mita <[email protected]>
>
> Reviewed-by: Alexander Potapenko <[email protected]>

2020-08-31 18:33:32

by Alexander Potapenko

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] add fault injection to user memory access

Hi Andrew,

On Mon, Aug 31, 2020 at 6:27 PM Akinobu Mita <[email protected]> wrote:
>
> Andrew,
>
> Could you take a look at this series, and consider taking in -mm tree?

Please consider picking v3 patches that address Peter's comments instead.

>
> 2020年9月1日(火) 0:49 Alexander Potapenko <[email protected]>:
> >
> > > This series looks good to me.
> >
> > Great!
> >
> > Which tree do fault injection patches normally go to?
> >
> > > Reviewed-by: Akinobu Mita <[email protected]>
> >
> > Reviewed-by: Alexander Potapenko <[email protected]>



--
Alexander Potapenko
Software Engineer

Google Germany GmbH
Erika-Mann-Straße, 33
80636 München

Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg

2020-08-31 19:46:26

by Alexander Potapenko

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] add fault injection to user memory access

> This series looks good to me.

Great!

Which tree do fault injection patches normally go to?

> Reviewed-by: Akinobu Mita <[email protected]>

Reviewed-by: Alexander Potapenko <[email protected]>

2020-08-31 20:25:40

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] lib, uaccess: add failure injection to usercopy functions

On Fri, Aug 28, 2020 at 02:13:43PM +0000, [email protected] wrote:
> @@ -82,6 +83,8 @@ __copy_from_user_inatomic(void *to, const void __user *from, unsigned long n)
> static __always_inline __must_check unsigned long
> __copy_from_user(void *to, const void __user *from, unsigned long n)
> {
> + if (should_fail_usercopy())
> + return n;
> might_fault();
> instrument_copy_from_user(to, from, n);
> check_object_size(to, n, false);

> @@ -124,7 +131,7 @@ _copy_from_user(void *to, const void __user *from, unsigned long n)
> {
> unsigned long res = n;
> might_fault();
> - if (likely(access_ok(from, n))) {
> + if (!should_fail_usercopy() && likely(access_ok(from, n))) {
> instrument_copy_from_user(to, from, n);
> res = raw_copy_from_user(to, from, n);
> }

You're inconsistent with your order against might_fault() throughout the
patch. After is the right place.