On Fri, Aug 21, 2020 at 01:51PM +0200, Dmitry Vyukov wrote:
...

quoted

+++ b/lib/fault-inject-usercopy.c

@@ -0,0 +1,66 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <linux/fault-inject.h>
+#include <linux/fault-inject-usercopy.h>
+#include <linux/random.h>
+
+static struct {
+       struct fault_attr attr;
+       u32 failsize;
+} fail_usercopy = {
+       .attr = FAULT_ATTR_INITIALIZER,
+       .failsize = 0,
+};
+
+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)
+{
+       umode_t mode = S_IFREG | 0600;
+       struct dentry *dir;
+
+       dir = fault_create_debugfs_attr("fail_usercopy", NULL,
+                                       &fail_usercopy.attr);
+       if (IS_ERR(dir))
+               return PTR_ERR(dir);
+
+       debugfs_create_u32("failsize", mode, dir,
+                          &fail_usercopy.failsize);

Marco, what's the right way to annotate these concurrent accesses for KCSAN?

For debugfs variables that are accessed concurrently, the only
non-data-racy option (currently) is to use debugfs_create_atomic_t() and
make the variable an atomic_t.

If it's read-mostly as is the case here, and given that atomic_read() is
cheap (it maps to READ_ONCE on x86 and arm64), that'd be reasonable even
if performance is a concern.

quoted

+       return 0;
+}
+
+late_initcall(fail_usercopy_debugfs);
+
+#endif /* CONFIG_FAULT_INJECTION_DEBUG_FS */
+
+/**
+ * should_fail_usercopy() - Failure code or amount of bytes not to copy.
+ * @n: Size of the original copy call.
+ *
+ * The general idea is to have a method which returns the amount of bytes not
+ * to copy, a failure to return, or 0 if the calling function should progress
+ * without a failure. E.g., copy_{to,from}_user should NOT copy the amount of
+ * bytes returned by should_fail_usercopy, returning this value (in addition
+ * to any bytes that could actually not be copied) or a failure.
+ *
+ * Return: one of:
+ * negative, failure to return;
+ * 0, progress normally;
+ * a number in ]0, n], the number of bytes not to copy.
+ *
+ */
+long should_fail_usercopy(unsigned long n)
+{
+       if (should_fail(&fail_usercopy.attr, n)) {
+               if (fail_usercopy.failsize > 0)
+                       return fail_usercopy.failsize % (n + 1);

If you wanted to retain the u32 in debugfs, you can mark this
'data_race(fail_usercopy.failsize)' -- since what we're doing here is
probabilistic anyway, reading a garbage value won't affect things much.

Alternatively, just switch to atomic_t and it'll just be an
atomic_read().

Thanks,
-- Marco