linux-kernel - Re: remove the last set_fs() in common code, and remove it for x86 and powerpc

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

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <319d15b1-cb4a-a7b4-3082-12bb30eb5143@csgroup.eu>
Date:   Tue, 18 Aug 2020 19:46:22 +0200
From:   Christophe Leroy <christophe.leroy@...roup.eu>
To:     Christoph Hellwig <hch@....de>, Al Viro <viro@...iv.linux.org.uk>,
        Michael Ellerman <mpe@...erman.id.au>, x86@...nel.org
Cc:     linux-fsdevel@...r.kernel.org, linux-arch@...r.kernel.org,
        linuxppc-dev@...ts.ozlabs.org, Kees Cook <keescook@...omium.org>,
        linux-kernel@...r.kernel.org
Subject: Re: remove the last set_fs() in common code, and remove it for x86
 and powerpc



Le 17/08/2020 à 09:32, Christoph Hellwig a écrit :
> Hi all,
> 
> this series removes the last set_fs() used to force a kernel address
> space for the uaccess code in the kernel read/write/splice code, and then
> stops implementing the address space overrides entirely for x86 and
> powerpc.
> 
> The file system part has been posted a few times, and the read/write side
> has been pretty much unchanced.  For splice this series drops the
> conversion of the seq_file and sysctl code to the iter ops, and thus loses
> the splice support for them.  The reasons for that is that it caused a lot
> of churn for not much use - splice for these small files really isn't much
> of a win, even if existing userspace uses it.  All callers I found do the
> proper fallback, but if this turns out to be an issue the conversion can
> be resurrected.

I like this series.

I gave it a go on my powerpc mpc832x. I tested it on top of my newest 
series that reworks the 32 bits signal handlers (see 
https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=196278) 
with the microbenchmark test used is that series.

With KUAP activated, on top of signal32 rework, performance is boosted 
as system time for the microbenchmark goes from 1.73s down to 1.56s, 
that is 10% quicker

Surprisingly, with the kernel as is today without my signal's series, 
your series degrades performance slightly (from 2.55s to 2.64s ie 3.5% 
slower).


I also observe, in both cases, a degradation on

	dd if=/dev/zero of=/dev/null count=1M

Without your series, it runs in 5.29 seconds.
With your series, it runs in 5.82 seconds, that is 10% more time.

Christophe


> 
> Besides x86 and powerpc I plan to eventually convert all other
> architectures, although this will be a slow process, starting with the
> easier ones once the infrastructure is merged.  The process to convert
> architectures is roughtly:
> 
>   - ensure there is no set_fs(KERNEL_DS) left in arch specific code
>   - implement __get_kernel_nofault and __put_kernel_nofault
>   - remove the arch specific address limitation functionality
> 
> Diffstat:
>   arch/Kconfig                           |    3
>   arch/alpha/Kconfig                     |    1
>   arch/arc/Kconfig                       |    1
>   arch/arm/Kconfig                       |    1
>   arch/arm64/Kconfig                     |    1
>   arch/c6x/Kconfig                       |    1
>   arch/csky/Kconfig                      |    1
>   arch/h8300/Kconfig                     |    1
>   arch/hexagon/Kconfig                   |    1
>   arch/ia64/Kconfig                      |    1
>   arch/m68k/Kconfig                      |    1
>   arch/microblaze/Kconfig                |    1
>   arch/mips/Kconfig                      |    1
>   arch/nds32/Kconfig                     |    1
>   arch/nios2/Kconfig                     |    1
>   arch/openrisc/Kconfig                  |    1
>   arch/parisc/Kconfig                    |    1
>   arch/powerpc/include/asm/processor.h   |    7 -
>   arch/powerpc/include/asm/thread_info.h |    5 -
>   arch/powerpc/include/asm/uaccess.h     |   78 ++++++++-----------
>   arch/powerpc/kernel/signal.c           |    3
>   arch/powerpc/lib/sstep.c               |    6 -
>   arch/riscv/Kconfig                     |    1
>   arch/s390/Kconfig                      |    1
>   arch/sh/Kconfig                        |    1
>   arch/sparc/Kconfig                     |    1
>   arch/um/Kconfig                        |    1
>   arch/x86/ia32/ia32_aout.c              |    1
>   arch/x86/include/asm/page_32_types.h   |   11 ++
>   arch/x86/include/asm/page_64_types.h   |   38 +++++++++
>   arch/x86/include/asm/processor.h       |   60 ---------------
>   arch/x86/include/asm/thread_info.h     |    2
>   arch/x86/include/asm/uaccess.h         |   26 ------
>   arch/x86/kernel/asm-offsets.c          |    3
>   arch/x86/lib/getuser.S                 |   28 ++++---
>   arch/x86/lib/putuser.S                 |   21 +++--
>   arch/xtensa/Kconfig                    |    1
>   drivers/char/mem.c                     |   16 ----
>   drivers/misc/lkdtm/bugs.c              |    2
>   drivers/misc/lkdtm/core.c              |    4 +
>   drivers/misc/lkdtm/usercopy.c          |    2
>   fs/read_write.c                        |   69 ++++++++++-------
>   fs/splice.c                            |  130 +++------------------------------
>   include/linux/fs.h                     |    2
>   include/linux/uaccess.h                |   18 ++++
>   lib/test_bitmap.c                      |   10 ++
>   46 files changed, 235 insertions(+), 332 deletions(-)
>