Re: [RFC PATCH] compat: update linux/compat.h and kernel/sys_ni.c

[Arnd Bergmann]

On Mon, Aug 22, 2022 at 9:43 PM Randy Dunlap <rdunlap@xxxxxxxxxxxxx> wrote:

>   * Deprecated system calls which are still defined in
> @@ -910,19 +901,13 @@ asmlinkage long compat_sys_old_select(st
> -#ifdef CONFIG_COMPAT_OLD_SIGACTION
> +struct compat_old_sigaction;
>  asmlinkage long compat_sys_sigaction(int sig,
>                                     const struct compat_old_sigaction __user *act,
>                                     struct compat_old_sigaction __user *oact);
> -#endif

All the removed #ifdef look good to me here, and I checked that there
are no conflicting prototypes for the ones you change.

For COND_SYSCALL_COMPAT(), I think they should not be part of
this patch, and only added for specific optional calls that would cause
a link failure.

> @@ -94,6 +94,9 @@ COND_SYSCALL(flock);
>  /* fs/nfsctl.c */
>
>  /* fs/open.c */
> +COND_SYSCALL_COMPAT(truncate64);
> +COND_SYSCALL_COMPAT(ftruncate64);
> +COND_SYSCALL_COMPAT(fallocate);

COND_SYSCALL_COMPAT() doesn't really make sense for non-optional syscalls
like these: if an architecture neither sets __ARCH_WANT_COMPAT_FALLOCATE
nor provides its own implementation, then a link failure is the
appropriate output, hiding it with a COND_SYSCALL_COMPAT() just turns that
into a runtime failure that is harder to analyse.

>  /* fs/read_write.c */
> +COND_SYSCALL_COMPAT(preadv64);
> +COND_SYSCALL_COMPAT(pwritev64);
> +COND_SYSCALL_COMPAT(pread64);
> +COND_SYSCALL_COMPAT(pwrite64);

These are specific to x32, and we don't want to ever add them to another
architecture, so they should not get a COND_SYSCALL_COMPAT()
either.

> @@ -118,6 +125,7 @@ COND_SYSCALL_COMPAT(signalfd4);
>  /* fs/sync.c */
> +COND_SYSCALL_COMPAT(sync_file_range);
....
> +COND_SYSCALL_COMPAT(readahead);

Same as above, but these have an additional angle to them, as
there are conflicting prototypes and implementations:

arch/sparc/kernel/systbls.h:long compat_sys_sync_file_range(unsigned int fd,
arch/sparc/kernel/systbls.h-                       unsigned off_high,
unsigned off_low,
arch/sparc/kernel/systbls.h-                       unsigned nb_high,
unsigned nb_low,
include/linux/compat.h:asmlinkage long compat_sys_sync_file_range(int
fd, compat_arg_u64(pos),
include/linux/compat.h-
compat_arg_u64(nbytes), unsigned int flags);

The current code works fine, but if you still want to improve this,
it would be great to convert the architecture specific helpers
to be shared with the common ones. For those that have non-matching
prototypes like

include/linux/compat.h:asmlinkage long compat_sys_pwrite64(unsigned
int fd, const char __user *buf, size_t count,
include/linux/compat.h-                             compat_arg_u64(pos));
arch/powerpc/include/asm/syscalls.h:compat_ssize_t
compat_sys_pwrite64(unsigned int fd, const char __user *ubuf,
compat_size_t count,
arch/powerpc/include/asm/syscalls.h-                               u32
reg6, u32 pos1, u32 pos2);

that have an extra argument in them, I would instead suggest renaming
the nonstandard ones.

       Arnd