diff mbox series

[01/13] syscalls: compat: Fix the missing part for __SYSCALL_COMPAT

Message ID 20211221163532.2636028-2-guoren@kernel.org (mailing list archive)
State New, archived
Headers show
Series riscv: compat: Add COMPAT mode support for rv64 | expand

Commit Message

Guo Ren Dec. 21, 2021, 4:35 p.m. UTC 
From: Guo Ren <guoren@linux.alibaba.com>

Make "uapi asm unistd.h" could be used for architectures' COMPAT
mode. The __SYSCALL_COMPAT is first used in riscv.

Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
---
 include/uapi/asm-generic/unistd.h       | 4 ++--
 tools/include/uapi/asm-generic/unistd.h | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

Comments

Arnd Bergmann Dec. 21, 2021, 5:08 p.m. UTC | #1 
On Tue, Dec 21, 2021 at 5:35 PM <guoren@kernel.org> wrote:
> diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h
> index 4557a8b6086f..aafe5cfeb27c 100644
> --- a/include/uapi/asm-generic/unistd.h
> +++ b/include/uapi/asm-generic/unistd.h
> @@ -383,7 +383,7 @@ __SYSCALL(__NR_syslog, sys_syslog)
>
>  /* kernel/ptrace.c */
>  #define __NR_ptrace 117
> -__SYSCALL(__NR_ptrace, sys_ptrace)
> +__SC_COMP(__NR_ptrace, sys_ptrace, compat_sys_ptrace)
>

Right. We could merge sys_ptrace and compat_sys_ptrace() by adding
a in_compat_syscall() check, but either way works.

>  /* kernel/sched/core.c */
>  #define __NR_sched_setparam 118
> @@ -779,7 +779,7 @@ __SYSCALL(__NR_rseq, sys_rseq)
>  #define __NR_kexec_file_load 294
>  __SYSCALL(__NR_kexec_file_load,     sys_kexec_file_load)
>  /* 295 through 402 are unassigned to sync up with generic numbers, don't use */
> -#if __BITS_PER_LONG == 32
> +#if defined(__SYSCALL_COMPAT) || __BITS_PER_LONG == 32
>  #define __NR_clock_gettime64 403
>  __SYSCALL(__NR_clock_gettime64, sys_clock_gettime)

This part looks wrong, you expose clock_gettime64 to user space this way, both
in asm/unistd.h and in the table.

         Arnd
Guo Ren Dec. 22, 2021, 11:16 a.m. UTC | #2 
On Wed, Dec 22, 2021 at 1:09 AM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Tue, Dec 21, 2021 at 5:35 PM <guoren@kernel.org> wrote:
> > diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h
> > index 4557a8b6086f..aafe5cfeb27c 100644
> > --- a/include/uapi/asm-generic/unistd.h
> > +++ b/include/uapi/asm-generic/unistd.h
> > @@ -383,7 +383,7 @@ __SYSCALL(__NR_syslog, sys_syslog)
> >
> >  /* kernel/ptrace.c */
> >  #define __NR_ptrace 117
> > -__SYSCALL(__NR_ptrace, sys_ptrace)
> > +__SC_COMP(__NR_ptrace, sys_ptrace, compat_sys_ptrace)
> >
>
> Right. We could merge sys_ptrace and compat_sys_ptrace() by adding
> a in_compat_syscall() check, but either way works.
>
> >  /* kernel/sched/core.c */
> >  #define __NR_sched_setparam 118
> > @@ -779,7 +779,7 @@ __SYSCALL(__NR_rseq, sys_rseq)
> >  #define __NR_kexec_file_load 294
> >  __SYSCALL(__NR_kexec_file_load,     sys_kexec_file_load)
> >  /* 295 through 402 are unassigned to sync up with generic numbers, don't use */
> > -#if __BITS_PER_LONG == 32
> > +#if defined(__SYSCALL_COMPAT) || __BITS_PER_LONG == 32
> >  #define __NR_clock_gettime64 403
> >  __SYSCALL(__NR_clock_gettime64, sys_clock_gettime)
>
> This part looks wrong, you expose clock_gettime64 to user space this way, both
> in asm/unistd.h and in the table.
>
>          Arnd

No, we only define __SYSCALL_COMPAT in compat_syscall_table.c. It
won't be expose to user space, because there is no __SYSCALL_COMPAT.

$ grep __SYSCALL_COMPAT * -r
arch/riscv/kernel/compat_syscall_table.c:#define __SYSCALL_COMPAT
                           ^^^^^^^^^^^^^^^^^^^^^^
include/uapi/asm-generic/unistd.h:#if __BITS_PER_LONG == 32 ||
defined(__SYSCALL_COMPAT)
include/uapi/asm-generic/unistd.h:#ifdef __SYSCALL_COMPAT
include/uapi/asm-generic/unistd.h:#if defined(__SYSCALL_COMPAT) ||
__BITS_PER_LONG == 32
include/uapi/asm-generic/unistd.h:#if __BITS_PER_LONG == 64 &&
!defined(__SYSCALL_COMPAT)
tools/include/uapi/asm-generic/unistd.h:#if __BITS_PER_LONG == 32 ||
defined(__SYSCALL_COMPAT)
tools/include/uapi/asm-generic/unistd.h:#ifdef __SYSCALL_COMPAT
tools/include/uapi/asm-generic/unistd.h:#if defined(__SYSCALL_COMPAT)
|| __BITS_PER_LONG == 32
tools/include/uapi/asm-generic/unistd.h:#if __BITS_PER_LONG == 64 &&
!defined(__SYSCALL_COMPAT)
Arnd Bergmann Dec. 22, 2021, 11:24 a.m. UTC | #3 
On Wed, Dec 22, 2021 at 12:16 PM Guo Ren <guoren@kernel.org> wrote:
> On Wed, Dec 22, 2021 at 1:09 AM Arnd Bergmann <arnd@arndb.de> wrote:
> > On Tue, Dec 21, 2021 at 5:35 PM <guoren@kernel.org> wrote:
> > >  /* 295 through 402 are unassigned to sync up with generic numbers, don't use */
> > > -#if __BITS_PER_LONG == 32
> > > +#if defined(__SYSCALL_COMPAT) || __BITS_PER_LONG == 32
> > >  #define __NR_clock_gettime64 403
> > >  __SYSCALL(__NR_clock_gettime64, sys_clock_gettime)
> >
> > This part looks wrong, you expose clock_gettime64 to user space this way, both
> > in asm/unistd.h and in the table.
> >
>
> No, we only define __SYSCALL_COMPAT in compat_syscall_table.c. It
> won't be expose to user space, because there is no __SYSCALL_COMPAT.

Ok, it looks good then. It feels like we should have a macro to wrap this, but
in reality what we should actually do is to convert the entire file to the
more modern syscall.tbl format, so let's just stay with your version.

      Arnd
Christoph Hellwig Dec. 24, 2021, 6:52 a.m. UTC | #4 
On Tue, Dec 21, 2021 at 06:08:45PM +0100, Arnd Bergmann wrote:
> > -__SYSCALL(__NR_ptrace, sys_ptrace)
> > +__SC_COMP(__NR_ptrace, sys_ptrace, compat_sys_ptrace)
> >
> 
> Right. We could merge sys_ptrace and compat_sys_ptrace() by adding
> a in_compat_syscall() check, but either way works.

I think merging them would be very useful, including merging
compat_arch_ptrace into arch_ptrace.  But we can leave that for
later.
diff mbox series

Patch

diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h
index 4557a8b6086f..aafe5cfeb27c 100644
--- a/include/uapi/asm-generic/unistd.h
+++ b/include/uapi/asm-generic/unistd.h
@@ -383,7 +383,7 @@  __SYSCALL(__NR_syslog, sys_syslog)
 
 /* kernel/ptrace.c */
 #define __NR_ptrace 117
-__SYSCALL(__NR_ptrace, sys_ptrace)
+__SC_COMP(__NR_ptrace, sys_ptrace, compat_sys_ptrace)
 
 /* kernel/sched/core.c */
 #define __NR_sched_setparam 118
@@ -779,7 +779,7 @@  __SYSCALL(__NR_rseq, sys_rseq)
 #define __NR_kexec_file_load 294
 __SYSCALL(__NR_kexec_file_load,     sys_kexec_file_load)
 /* 295 through 402 are unassigned to sync up with generic numbers, don't use */
-#if __BITS_PER_LONG == 32
+#if defined(__SYSCALL_COMPAT) || __BITS_PER_LONG == 32
 #define __NR_clock_gettime64 403
 __SYSCALL(__NR_clock_gettime64, sys_clock_gettime)
 #define __NR_clock_settime64 404
diff --git a/tools/include/uapi/asm-generic/unistd.h b/tools/include/uapi/asm-generic/unistd.h
index 4557a8b6086f..aafe5cfeb27c 100644
--- a/tools/include/uapi/asm-generic/unistd.h
+++ b/tools/include/uapi/asm-generic/unistd.h
@@ -383,7 +383,7 @@  __SYSCALL(__NR_syslog, sys_syslog)
 
 /* kernel/ptrace.c */
 #define __NR_ptrace 117
-__SYSCALL(__NR_ptrace, sys_ptrace)
+__SC_COMP(__NR_ptrace, sys_ptrace, compat_sys_ptrace)
 
 /* kernel/sched/core.c */
 #define __NR_sched_setparam 118
@@ -779,7 +779,7 @@  __SYSCALL(__NR_rseq, sys_rseq)
 #define __NR_kexec_file_load 294
 __SYSCALL(__NR_kexec_file_load,     sys_kexec_file_load)
 /* 295 through 402 are unassigned to sync up with generic numbers, don't use */
-#if __BITS_PER_LONG == 32
+#if defined(__SYSCALL_COMPAT) || __BITS_PER_LONG == 32
 #define __NR_clock_gettime64 403
 __SYSCALL(__NR_clock_gettime64, sys_clock_gettime)
 #define __NR_clock_settime64 404