linux-kernel - Re: [PATCH] asm-generic: unistd.h: make 'compat_sys_fadvise64

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]

Date:   Sun, 7 Aug 2022 21:44:43 +0200
From:   Arnd Bergmann <arnd@...db.de>
To:     Randy Dunlap <rdunlap@...radead.org>
Cc:     linux-kernel@...r.kernel.org,
        Paul Walmsley <paul.walmsley@...ive.com>,
        Palmer Dabbelt <palmer@...belt.com>,
        Albert Ou <aou@...s.berkeley.edu>,
        linux-riscv@...ts.infradead.org, Arnd Bergmann <arnd@...db.de>,
        linux-arch@...r.kernel.org,
        Andrew Morton <akpm@...ux-foundation.org>, linux-mm@...ck.org
Subject: Re: [PATCH] asm-generic: unistd.h: make 'compat_sys_fadvise64_64' conditional

On Sun, Aug 7, 2022 at 7:28 PM Randy Dunlap <rdunlap@...radead.org> wrote:
>
> Don't require 'compat_sys_fadvise64_64' when
> __ARCH_WANT_COMPAT_FADVISE64_64 is not set.
>
> Fixes this build error when CONFIG_ADVISE_SYSCALLS is not set:
>
> include/uapi/asm-generic/unistd.h:649:49: error: 'compat_sys_fadvise64_64' undeclared here (not in a function); did you mean 'ksys_fadvise64_64'?
>   649 | __SC_COMP(__NR3264_fadvise64, sys_fadvise64_64, compat_sys_fadvise64_64)
> arch/riscv/kernel/compat_syscall_table.c:12:42: note: in definition of macro '__SYSCALL'
>    12 | #define __SYSCALL(nr, call)      [nr] = (call),
> include/uapi/asm-generic/unistd.h:649:1: note: in expansion of macro '__SC_COMP'
>   649 | __SC_COMP(__NR3264_fadvise64, sys_fadvise64_64, compat_sys_fadvise64_64)
>
> Signed-off-by: Randy Dunlap <rdunlap@...radead.org>
> Cc: Paul Walmsley <paul.walmsley@...ive.com>
> Cc: Palmer Dabbelt <palmer@...belt.com>
> Cc: Albert Ou <aou@...s.berkeley.edu>
> Cc: linux-riscv@...ts.infradead.org
> Cc: Arnd Bergmann <arnd@...db.de>
> Cc: linux-arch@...r.kernel.org
> Cc: Andrew Morton <akpm@...ux-foundation.org>
> Cc: linux-mm@...ck.org
> ---
>  include/uapi/asm-generic/unistd.h |    2 ++
>  1 file changed, 2 insertions(+)
>
> --- a/include/uapi/asm-generic/unistd.h
> +++ b/include/uapi/asm-generic/unistd.h
> @@ -645,8 +645,10 @@ __SC_COMP(__NR_execve, sys_execve, compa
>  #define __NR3264_mmap 222
>  __SC_3264(__NR3264_mmap, sys_mmap2, sys_mmap)
>  /* mm/fadvise.c */
> +#ifdef __ARCH_WANT_COMPAT_FADVISE64_64
>  #define __NR3264_fadvise64 223
>  __SC_COMP(__NR3264_fadvise64, sys_fadvise64_64, compat_sys_fadvise64_64)
> +#endif
>

This does not work: __ARCH_WANT_COMPAT_FADVISE64_64 is defined in
arch/riscv/include/asm/unistd.h, which is not a UAPI header. By making the line
conditional on this, user space no longer sees the macro definition.

It looks like you also drop the native definition on all architectures other
than riscv here. What we probably want is to just make all the
declarations in include/linux/compat.h unconditional and not have them
depend on architecture specific macros. Some of these may have
incompatible prototypes depending on the architecture, but if we run
into those, I would suggest we just give them unique names.

       Arnd