linux-kernel - Re: [PATCH V3 08/17] riscv: compat: syscall: Add compat_sys_call

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <CAK8P3a0LxB3we9wHOa4OPmNow6wz5NP49zeYhh7QXNv-MiR8UA@mail.gmail.com>
Date:   Thu, 20 Jan 2022 15:42:42 +0100
From:   Arnd Bergmann <arnd@...db.de>
To:     Guo Ren <guoren@...nel.org>
Cc:     Palmer Dabbelt <palmer@...belt.com>, Arnd Bergmann <arnd@...db.de>,
        Anup Patel <anup@...infault.org>,
        gregkh <gregkh@...uxfoundation.org>,
        liush <liush@...winnertech.com>, Wei Fu <wefu@...hat.com>,
        Drew Fustini <drew@...gleboard.org>,
        Wang Junqiang <wangjunqiang@...as.ac.cn>,
        Christoph Hellwig <hch@....de>,
        Christoph Hellwig <hch@...radead.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-riscv <linux-riscv@...ts.infradead.org>,
        linux-csky@...r.kernel.org,
        linux-s390 <linux-s390@...r.kernel.org>,
        sparclinux <sparclinux@...r.kernel.org>,
        linuxppc-dev <linuxppc-dev@...ts.ozlabs.org>,
        inux-parisc@...r.kernel.org,
        "open list:BROADCOM NVRAM DRIVER" <linux-mips@...r.kernel.org>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        "the arch/x86 maintainers" <x86@...nel.org>,
        Guo Ren <guoren@...ux.alibaba.com>
Subject: Re: [PATCH V3 08/17] riscv: compat: syscall: Add compat_sys_call_table
 implementation

On Thu, Jan 20, 2022 at 8:39 AM <guoren@...nel.org> wrote:
>
>  /* The array of function pointers for syscalls. */
>  extern void * const sys_call_table[];
> +#ifdef CONFIG_COMPAT
> +extern void * const compat_sys_call_table[];
> +#endif

No need for the #ifdef, the normal convention is to just define the
extern declaration unconditionally for symbols that may or may not be defined.

> +COMPAT_SYSCALL_DEFINE3(truncate64, const char __user *, pathname,
> +                      arg_u32p(length))
> +{
> +       return ksys_truncate(pathname, arg_u64(length));
> +}

Are you sure these are the right calling conventions? According to [1],
I think the 64-bit argument should be in an aligned pair of registers,
which means you need an extra pad argument as in the arm64 version
of these functions. Same for ftruncate64, pread64, pwrite64, and
readahead.

> +COMPAT_SYSCALL_DEFINE3(ftruncate64, unsigned int, fd, arg_u32p(length))
> +{
> +       return ksys_ftruncate(fd, arg_u64(length));
> +}
> +
> +COMPAT_SYSCALL_DEFINE6(fallocate, int, fd, int, mode,
> +                      arg_u32p(offset), arg_u32p(len))
> +{
> +       return ksys_fallocate(fd, mode, arg_u64(offset), arg_u64(len));
> +}
> +
> +COMPAT_SYSCALL_DEFINE5(pread64, unsigned int, fd, char __user *, buf,
> +                      size_t, count, arg_u32p(pos))
> +{
> +       return ksys_pread64(fd, buf, count, arg_u64(pos));
> +}
> +
> +COMPAT_SYSCALL_DEFINE5(pwrite64, unsigned int, fd,
> +                      const char __user *, buf, size_t, count, arg_u32p(pos))
> +{
> +       return ksys_pwrite64(fd, buf, count, arg_u64(pos));
> +}
> +
> +COMPAT_SYSCALL_DEFINE6(sync_file_range, int, fd, arg_u32p(offset),
> +                      arg_u32p(nbytes), unsigned int, flags)
> +{
> +       return ksys_sync_file_range(fd, arg_u64(offset), arg_u64(nbytes),
> +                                   flags);
> +}
> +
> +COMPAT_SYSCALL_DEFINE4(readahead, int, fd, arg_u32p(offset),
> +                      size_t, count)
> +{
> +       return ksys_readahead(fd, arg_u64(offset), count);
> +}
> +
> +COMPAT_SYSCALL_DEFINE6(fadvise64_64, int, fd, int, advice, arg_u32p(offset),
> +                      arg_u32p(len))
> +{
> +       return ksys_fadvise64_64(fd, arg_u64(offset), arg_u64(len), advice);
> +}

I still feel like these should be the common implementations next to the
native handlers inside of an #ifdef CONFIG_COMPAT.

The names clash with the custom versions defined for powerpc and sparc,
but the duplicates look compatible if you can account for the padded
argument and the lo/hi order of the pairs, so could just be removed here
(all other architectures use custom function names instead).

        Arnd

[1] https://riscv.org/wp-content/uploads/2015/01/riscv-calling.pdf