2020-12-20 10:00:50

by Heiko Carstens

[permalink] [raw]
Subject: [PATCH] epoll: fix compat syscall wire up of epoll_pwait2

Commit b0a0c2615f6f ("epoll: wire up syscall epoll_pwait2") wired up
the 64 bit syscall instead of the compat variant in a couple of places.

Cc: Willem de Bruijn <[email protected]>
Cc: Al Viro <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Matthew Wilcox (Oracle) <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Thomas Bogendoerfer <[email protected]>
Cc: Vasily Gorbik <[email protected]>
Cc: Christian Borntraeger <[email protected]>
Cc: "David S. Miller" <[email protected]>
Fixes: b0a0c2615f6f ("epoll: wire up syscall epoll_pwait2")
Signed-off-by: Heiko Carstens <[email protected]>
---
arch/arm64/include/asm/unistd32.h | 2 +-
arch/mips/kernel/syscalls/syscall_n32.tbl | 2 +-
arch/s390/kernel/syscalls/syscall.tbl | 2 +-
arch/sparc/kernel/syscalls/syscall.tbl | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/include/asm/unistd32.h b/arch/arm64/include/asm/unistd32.h
index f4bca2b90218..cccfbbefbf95 100644
--- a/arch/arm64/include/asm/unistd32.h
+++ b/arch/arm64/include/asm/unistd32.h
@@ -890,7 +890,7 @@ __SYSCALL(__NR_faccessat2, sys_faccessat2)
#define __NR_process_madvise 440
__SYSCALL(__NR_process_madvise, sys_process_madvise)
#define __NR_epoll_pwait2 441
-__SYSCALL(__NR_epoll_pwait2, sys_epoll_pwait2)
+__SYSCALL(__NR_epoll_pwait2, compat_sys_epoll_pwait2)

/*
* Please add new compat syscalls above this comment and update
diff --git a/arch/mips/kernel/syscalls/syscall_n32.tbl b/arch/mips/kernel/syscalls/syscall_n32.tbl
index ad9c3dd0ab1f..0f03ad223f33 100644
--- a/arch/mips/kernel/syscalls/syscall_n32.tbl
+++ b/arch/mips/kernel/syscalls/syscall_n32.tbl
@@ -379,4 +379,4 @@
438 n32 pidfd_getfd sys_pidfd_getfd
439 n32 faccessat2 sys_faccessat2
440 n32 process_madvise sys_process_madvise
-441 n32 epoll_pwait2 sys_epoll_pwait2
+441 n32 epoll_pwait2 compat_sys_epoll_pwait2
diff --git a/arch/s390/kernel/syscalls/syscall.tbl b/arch/s390/kernel/syscalls/syscall.tbl
index 14f6525886a8..d443423495e5 100644
--- a/arch/s390/kernel/syscalls/syscall.tbl
+++ b/arch/s390/kernel/syscalls/syscall.tbl
@@ -443,4 +443,4 @@
438 common pidfd_getfd sys_pidfd_getfd sys_pidfd_getfd
439 common faccessat2 sys_faccessat2 sys_faccessat2
440 common process_madvise sys_process_madvise sys_process_madvise
-441 common epoll_pwait2 sys_epoll_pwait2 sys_epoll_pwait2
+441 common epoll_pwait2 sys_epoll_pwait2 compat_sys_epoll_pwait2
diff --git a/arch/sparc/kernel/syscalls/syscall.tbl b/arch/sparc/kernel/syscalls/syscall.tbl
index c7da4c3271e6..40d8c7cd8298 100644
--- a/arch/sparc/kernel/syscalls/syscall.tbl
+++ b/arch/sparc/kernel/syscalls/syscall.tbl
@@ -486,4 +486,4 @@
438 common pidfd_getfd sys_pidfd_getfd
439 common faccessat2 sys_faccessat2
440 common process_madvise sys_process_madvise
-441 common epoll_pwait2 sys_epoll_pwait2
+441 common epoll_pwait2 sys_epoll_pwait2 compat_sys_epoll_pwait2
--
2.17.1


2020-12-20 11:45:09

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH] epoll: fix compat syscall wire up of epoll_pwait2

On Sun, Dec 20, 2020 at 11:00 AM Heiko Carstens <[email protected]> wrote:
>
> Commit b0a0c2615f6f ("epoll: wire up syscall epoll_pwait2") wired up
> the 64 bit syscall instead of the compat variant in a couple of places.
>
> Cc: Willem de Bruijn <[email protected]>
> Cc: Al Viro <[email protected]>
> Cc: Arnd Bergmann <[email protected]>
> Cc: Matthew Wilcox (Oracle) <[email protected]>
> Cc: Catalin Marinas <[email protected]>
> Cc: Will Deacon <[email protected]>
> Cc: Thomas Bogendoerfer <[email protected]>
> Cc: Vasily Gorbik <[email protected]>
> Cc: Christian Borntraeger <[email protected]>
> Cc: "David S. Miller" <[email protected]>
> Fixes: b0a0c2615f6f ("epoll: wire up syscall epoll_pwait2")
> Signed-off-by: Heiko Carstens <[email protected]>
> ---
> arch/arm64/include/asm/unistd32.h | 2 +-
> arch/mips/kernel/syscalls/syscall_n32.tbl | 2 +-
> arch/s390/kernel/syscalls/syscall.tbl | 2 +-
> arch/sparc/kernel/syscalls/syscall.tbl | 2 +-
> 4 files changed, 4 insertions(+), 4 deletions(-)

I double-checked all the entries to make sure you caught all
the missing ones, looks good.

Acked-by: Arnd Bergmann <[email protected]>

2020-12-20 18:25:09

by Willem de Bruijn

[permalink] [raw]
Subject: Re: [PATCH] epoll: fix compat syscall wire up of epoll_pwait2

On Sun, Dec 20, 2020 at 6:43 AM Arnd Bergmann <[email protected]> wrote:
>
> On Sun, Dec 20, 2020 at 11:00 AM Heiko Carstens <[email protected]> wrote:
> >
> > Commit b0a0c2615f6f ("epoll: wire up syscall epoll_pwait2") wired up
> > the 64 bit syscall instead of the compat variant in a couple of places.
> >
> > Cc: Willem de Bruijn <[email protected]>
> > Cc: Al Viro <[email protected]>
> > Cc: Arnd Bergmann <[email protected]>
> > Cc: Matthew Wilcox (Oracle) <[email protected]>
> > Cc: Catalin Marinas <[email protected]>
> > Cc: Will Deacon <[email protected]>
> > Cc: Thomas Bogendoerfer <[email protected]>
> > Cc: Vasily Gorbik <[email protected]>
> > Cc: Christian Borntraeger <[email protected]>
> > Cc: "David S. Miller" <[email protected]>
> > Fixes: b0a0c2615f6f ("epoll: wire up syscall epoll_pwait2")
> > Signed-off-by: Heiko Carstens <[email protected]>
> > ---
> > arch/arm64/include/asm/unistd32.h | 2 +-
> > arch/mips/kernel/syscalls/syscall_n32.tbl | 2 +-
> > arch/s390/kernel/syscalls/syscall.tbl | 2 +-
> > arch/sparc/kernel/syscalls/syscall.tbl | 2 +-
> > 4 files changed, 4 insertions(+), 4 deletions(-)
>
> I double-checked all the entries to make sure you caught all
> the missing ones, looks good.
>
> Acked-by: Arnd Bergmann <[email protected]>

Acked-by: Willem de Bruijn <[email protected]>

Thanks a lot. I also arrived at the same list after comparing to
epoll_pwait and signalfd (for sigset) and ppoll_time64 (for timespec64).

Slightly tangential, it's not immediately clear to me why in
arch/x86/entry/syscalls/syscall_32.tbl epoll_pwait does not need a
compat entry, unlike on other architectures and unlike signalfd.

2020-12-20 20:16:46

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH] epoll: fix compat syscall wire up of epoll_pwait2

On Sun, Dec 20, 2020 at 7:51 PM Linus Torvalds
<[email protected]> wrote:
>
> On Sun, Dec 20, 2020 at 10:22 AM Willem de Bruijn
> <[email protected]> wrote:
> >
> > Slightly tangential, it's not immediately clear to me why in
> > arch/x86/entry/syscalls/syscall_32.tbl epoll_pwait does not need a
> > compat entry, unlike on other architectures and unlike signalfd.
>
> Hmm. Good question. That looks like a bug to me. Probably nobody
> noticed because it's so rarely used.
>
> Or maybe I'm missing something too.
>
> Adding x86 entry code people to the participants.

The sigset_t argument is actually compatible between x86-32 and x86-64
because

- The bits are in the same order on little-endian machines
- _NSIG is the same as _COMPAT_NSIG (unlike old sparc kernels)
- accessing a 64-bit with 32-bit alignment is always allowed on x86

All other architectures with compat mode support big-endian
code at least as an option, so they have to use the compat
version.

Arnd

2020-12-20 23:21:41

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH] epoll: fix compat syscall wire up of epoll_pwait2

On Sun, Dec 20, 2020 at 10:06 PM Linus Torvalds
<[email protected]> wrote:
>
> On Sun, Dec 20, 2020 at 12:14 PM Arnd Bergmann <[email protected]> wrote:
> >
> > The sigset_t argument is actually compatible between x86-32 and x86-64
>
> Well, random high bits in size_t or the pointer value aren't. So it
> still looks a bit iffy to me.
>
> But it might end up working almost by accident.

The direct syscall arguments all get the correct zero-extension with
asm/syscall_wrapper.h, just like they do with any other syscall that
is used in both native and compat mode, like epoll_wait().

It probably makes sense to change it just for consistency with the other
architectures, but I would assume that it was intentional when this was
added originally, as the compat handling for epoll_pwait() went through
several iterations before it first worked correctly.

Arnd

2020-12-21 05:33:07

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH] epoll: fix compat syscall wire up of epoll_pwait2

On Sun, Dec 20, 2020 at 12:14 PM Arnd Bergmann <[email protected]> wrote:
>
> The sigset_t argument is actually compatible between x86-32 and x86-64

Well, random high bits in size_t or the pointer value aren't. So it
still looks a bit iffy to me.

But it might end up working almost by accident.

Linus

2020-12-21 05:40:58

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH] epoll: fix compat syscall wire up of epoll_pwait2

On Sun, Dec 20, 2020 at 10:22 AM Willem de Bruijn
<[email protected]> wrote:
>
> Slightly tangential, it's not immediately clear to me why in
> arch/x86/entry/syscalls/syscall_32.tbl epoll_pwait does not need a
> compat entry, unlike on other architectures and unlike signalfd.

Hmm. Good question. That looks like a bug to me. Probably nobody
noticed because it's so rarely used.

Or maybe I'm missing something too.

Adding x86 entry code people to the participants.

Linus

2020-12-23 14:50:09

by Will Deacon

[permalink] [raw]
Subject: Re: [PATCH] epoll: fix compat syscall wire up of epoll_pwait2

On Sun, Dec 20, 2020 at 10:58:30AM +0100, Heiko Carstens wrote:
> Commit b0a0c2615f6f ("epoll: wire up syscall epoll_pwait2") wired up
> the 64 bit syscall instead of the compat variant in a couple of places.
>
> Cc: Willem de Bruijn <[email protected]>
> Cc: Al Viro <[email protected]>
> Cc: Arnd Bergmann <[email protected]>
> Cc: Matthew Wilcox (Oracle) <[email protected]>
> Cc: Catalin Marinas <[email protected]>
> Cc: Will Deacon <[email protected]>
> Cc: Thomas Bogendoerfer <[email protected]>
> Cc: Vasily Gorbik <[email protected]>
> Cc: Christian Borntraeger <[email protected]>
> Cc: "David S. Miller" <[email protected]>
> Fixes: b0a0c2615f6f ("epoll: wire up syscall epoll_pwait2")
> Signed-off-by: Heiko Carstens <[email protected]>
> ---
> arch/arm64/include/asm/unistd32.h | 2 +-
> arch/mips/kernel/syscalls/syscall_n32.tbl | 2 +-
> arch/s390/kernel/syscalls/syscall.tbl | 2 +-
> arch/sparc/kernel/syscalls/syscall.tbl | 2 +-
> 4 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/include/asm/unistd32.h b/arch/arm64/include/asm/unistd32.h
> index f4bca2b90218..cccfbbefbf95 100644
> --- a/arch/arm64/include/asm/unistd32.h
> +++ b/arch/arm64/include/asm/unistd32.h
> @@ -890,7 +890,7 @@ __SYSCALL(__NR_faccessat2, sys_faccessat2)
> #define __NR_process_madvise 440
> __SYSCALL(__NR_process_madvise, sys_process_madvise)
> #define __NR_epoll_pwait2 441
> -__SYSCALL(__NR_epoll_pwait2, sys_epoll_pwait2)
> +__SYSCALL(__NR_epoll_pwait2, compat_sys_epoll_pwait2)


For this bit ^^

Acked-by: Will Deacon <[email protected]>

Will