lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Fri, 21 Aug 2020 15:31:20 +0200 From: Marco Elver <elver@...gle.com> To: Dmitry Vyukov <dvyukov@...gle.com> Cc: albert.linde@...il.com, Andrew Morton <akpm@...ux-foundation.org>, Borislav Petkov <bp@...en8.de>, Ingo Molnar <mingo@...hat.com>, Jonathan Corbet <corbet@....net>, Thomas Gleixner <tglx@...utronix.de>, Arnd Bergmann <arnd@...db.de>, Akinobu Mita <akinobu.mita@...il.com>, "H. Peter Anvin" <hpa@...or.com>, Al Viro <viro@...iv.linux.org.uk>, Alexander Potapenko <glider@...gle.com>, Andrey Konovalov <andreyknvl@...gle.com>, "open list:DOCUMENTATION" <linux-doc@...r.kernel.org>, LKML <linux-kernel@...r.kernel.org>, linux-arch <linux-arch@...r.kernel.org>, the arch/x86 maintainers <x86@...nel.org>, Albert van der Linde <alinde@...gle.com> Subject: Re: [PATCH 1/3] lib, include/linux: add usercopy failure capability On Fri, Aug 21, 2020 at 01:51PM +0200, Dmitry Vyukov wrote: ... > > +++ 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. > > + 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
Powered by blists - more mailing lists