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, 18 Feb 2022 10:20:38 +0100 From: Arnd Bergmann <arnd@...nel.org> To: Al Viro <viro@...iv.linux.org.uk> Cc: Christophe Leroy <christophe.leroy@...roup.eu>, Linus Torvalds <torvalds@...ux-foundation.org>, Christoph Hellwig <hch@....de>, "linux-arch@...r.kernel.org" <linux-arch@...r.kernel.org>, "linux-mm@...ck.org" <linux-mm@...ck.org>, "linux-api@...r.kernel.org" <linux-api@...r.kernel.org>, "arnd@...db.de" <arnd@...db.de>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, "mark.rutland@....com" <mark.rutland@....com>, "dalias@...c.org" <dalias@...c.org>, "linux-ia64@...r.kernel.org" <linux-ia64@...r.kernel.org>, "linux-sh@...r.kernel.org" <linux-sh@...r.kernel.org>, "peterz@...radead.org" <peterz@...radead.org>, "jcmvbkbc@...il.com" <jcmvbkbc@...il.com>, "guoren@...nel.org" <guoren@...nel.org>, "sparclinux@...r.kernel.org" <sparclinux@...r.kernel.org>, "linux-hexagon@...r.kernel.org" <linux-hexagon@...r.kernel.org>, "linux-riscv@...ts.infradead.org" <linux-riscv@...ts.infradead.org>, "will@...nel.org" <will@...nel.org>, "ardb@...nel.org" <ardb@...nel.org>, "linux-s390@...r.kernel.org" <linux-s390@...r.kernel.org>, "bcain@...eaurora.org" <bcain@...eaurora.org>, "deller@....de" <deller@....de>, "x86@...nel.org" <x86@...nel.org>, "linux@...linux.org.uk" <linux@...linux.org.uk>, "linux-csky@...r.kernel.org" <linux-csky@...r.kernel.org>, "mingo@...hat.com" <mingo@...hat.com>, "geert@...ux-m68k.org" <geert@...ux-m68k.org>, "linux-snps-arc@...ts.infradead.org" <linux-snps-arc@...ts.infradead.org>, "linux-xtensa@...ux-xtensa.org" <linux-xtensa@...ux-xtensa.org>, "hca@...ux.ibm.com" <hca@...ux.ibm.com>, "linux-alpha@...r.kernel.org" <linux-alpha@...r.kernel.org>, "linux-um@...ts.infradead.org" <linux-um@...ts.infradead.org>, "linux-m68k@...ts.linux-m68k.org" <linux-m68k@...ts.linux-m68k.org>, "openrisc@...ts.librecores.org" <openrisc@...ts.librecores.org>, "green.hu@...il.com" <green.hu@...il.com>, "shorne@...il.com" <shorne@...il.com>, "monstr@...str.eu" <monstr@...str.eu>, "tsbogend@...ha.franken.de" <tsbogend@...ha.franken.de>, "linux-parisc@...r.kernel.org" <linux-parisc@...r.kernel.org>, "nickhu@...estech.com" <nickhu@...estech.com>, "linux-mips@...r.kernel.org" <linux-mips@...r.kernel.org>, "dinguyen@...nel.org" <dinguyen@...nel.org>, "ebiederm@...ssion.com" <ebiederm@...ssion.com>, "richard@....at" <richard@....at>, "akpm@...ux-foundation.org" <akpm@...ux-foundation.org>, "linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>, "davem@...emloft.net" <davem@...emloft.net> Subject: Re: [PATCH v2 00/18] clean up asm/uaccess.h, kill set_fs for good n Fri, Feb 18, 2022 at 3:21 AM Al Viro <viro@...iv.linux.org.uk> wrote: > > On Thu, Feb 17, 2022 at 08:49:59AM +0100, Arnd Bergmann wrote: > > > Same here: architectures can already provide a __put_user_fn() > > and __get_user_fn(), to get the generic versions of the interface, > > but few architectures use that. You can actually get all the interfaces > > by just providing raw_copy_from_user() and raw_copy_to_user(), > > but the get_user/put_user versions you get from that are fairly > > inefficient. > > FWIW, __{get,put}_user_{8,16,32,64} would probably make it easier to > unify. That's where the really variable part tends to be, anyway. > IMO __get_user_fn() had been a mistake. I've prototyped this now, to see what this might look like, see https://git.kernel.org/pub/scm/linux/kernel/git/arnd/playground.git/commit/?h=generic-get_user-prototype This adds generic inline version of {__get,get,__put,put}_user() and converts x86 to (optionally) use it. This builds with gcc-5 through gcc-11 on 32-bit and 64-bit x86, using asm-goto with outputs where possible, and requiring a minimum set of macro definitions from the architecture. Compiling with clang produces no warnings but does cause a linker issue at the moment, so there is probably at least one bug in it. Aside from compile-testing, I have not tried to verify if this is correct or efficient, but let me know if you think this is headed in the right direction. > One thing I somewhat dislike about the series is the boilerplate in > asm/uaccess.h instances - #include <asm-generic/access-ok.h> in > a lot of them might make sense as a transitory state, but getting > stuck with those indefinitely... Christoph also complained about it, the problem for now is that asm-generic/access_ok.h must first see the macro definitions for architectures that override any of the contents, but access_ok() itself is used at least in some of the asm/uaccess.h files as well, so it must be included in the middle of it, until more of the uaccess.h implementation is moved to linux/uaccess.h in an architecture independent way. Would you prefer having an asm/access_ok.h that falls back to the asm-generic version but can have an architecture specific override when needed (ia64, arm64, x86, um)? > BTW, do we need user_addr_max() anymore? The definition in > asm-generic/access-ok.h is the only one, so ifndef around it is pointless. Right, the v2 changes got rid of the last override, so it could get hardcoded to TASK_SIZE_MAX, or we can convert the five references to just use that instead and remove it altogether: arch/arm64/kernel/traps.c: if (address >= user_addr_max()) { \ arch/parisc/kernel/signal.c: if (start >= user_addr_max() - sigframe_size) arch/parisc/kernel/signal.c: if (A(&usp[0]) >= user_addr_max() - 5 * sizeof(int)) lib/strncpy_from_user.c: max_addr = user_addr_max(); lib/strnlen_user.c: max_addr = user_addr_max(); user_addr_max() first showed up in architecture-independent code in c5389831cda3 ("sparc: Fix user_addr_max() definition."), and from that I think the original intent is no longer useful. Arnd
Powered by blists - more mailing lists