2019-09-05 18:39:12

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH 1/2] ipc: fix semtimedop for generic 32-bit architectures

As Vincent noticed, the y2038 conversion of semtimedop in linux-5.1
broke when commit 00bf25d693e7 ("y2038: use time32 syscall names on
32-bit") changed all system calls on all architectures that take
a 32-bit time_t to point to the _time32 implementation, but left out
semtimedop in the asm-generic header.

This affects all 32-bit architectures using asm-generic/unistd.h:
h8300, unicore32, openrisc, nios2, hexagon, c6x, arc, nds32 and csky.

The notable exception is riscv32, which has dropped support for the
time32 system calls entirely.

Reported-by: Vincent Chen <[email protected]>
Cc: [email protected]
Cc: Vincent Chen <[email protected]>
Cc: Greentime Hu <[email protected]>
Cc: Yoshinori Sato <[email protected]>
Cc: Guan Xuetao <[email protected]>
Cc: Stafford Horne <[email protected]>
Cc: Jonas Bonn <[email protected]>
Cc: Stefan Kristiansson <[email protected]>
Cc: Ley Foon Tan <[email protected]>
Cc: Richard Kuo <[email protected]>
Cc: Mark Salter <[email protected]>
Cc: Aurelien Jacquiot <[email protected]>
Cc: Guo Ren <[email protected]>
Fixes: 00bf25d693e7 ("y2038: use time32 syscall names on 32-bit")
Signed-off-by: Arnd Bergmann <[email protected]>
---
Hi Vincent,

Sorry for the delay since your report. Does this address your
problem?

Anyone else, please note that this patch is required since
5.1 to make sysvipc work on the listed architectures.
---
include/uapi/asm-generic/unistd.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h
index 1be0e798e362..1fc8faa6e973 100644
--- a/include/uapi/asm-generic/unistd.h
+++ b/include/uapi/asm-generic/unistd.h
@@ -569,7 +569,7 @@ __SYSCALL(__NR_semget, sys_semget)
__SC_COMP(__NR_semctl, sys_semctl, compat_sys_semctl)
#if defined(__ARCH_WANT_TIME32_SYSCALLS) || __BITS_PER_LONG != 32
#define __NR_semtimedop 192
-__SC_COMP(__NR_semtimedop, sys_semtimedop, sys_semtimedop_time32)
+__SC_3264(__NR_semtimedop, sys_semtimedop_time32, sys_semtimedop)
#endif
#define __NR_semop 193
__SYSCALL(__NR_semop, sys_semop)
--
2.20.0


2019-09-05 18:41:08

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH 2/2] ipc: fix sparc64 ipc() wrapper

Matt bisected a sparc64 specific issue with semctl, shmctl and msgctl
to a commit from my y2038 series in linux-5.1, as I missed the custom
sys_ipc() wrapper that sparc64 uses in place of the generic version that
I patched.

The problem is that the sys_{sem,shm,msg}ctl() functions in the kernel
now do not allow being called with the IPC_64 flag any more, resulting
in a -EINVAL error when they don't recognize the command.

Instead, the correct way to do this now is to call the internal
ksys_old_{sem,shm,msg}ctl() functions to select the API version.

As we generally move towards these functions anyway, change all of
sparc_ipc() to consistently use those in place of the sys_*() versions,
and move the required ksys_*() declarations into linux/syscalls.h

Reported-by: Matt Turner <[email protected]>
Fixes: 275f22148e87 ("ipc: rename old-style shmctl/semctl/msgctl syscalls")
Cc: [email protected]
Signed-off-by: Arnd Bergmann <[email protected]>
---
Hi Matt,

Can you check that this solves your problem?
---
arch/sparc/kernel/sys_sparc_64.c | 30 +++++++++++++++---------------
include/linux/syscalls.h | 19 +++++++++++++++++++
ipc/util.h | 25 ++-----------------------
3 files changed, 36 insertions(+), 38 deletions(-)

diff --git a/arch/sparc/kernel/sys_sparc_64.c b/arch/sparc/kernel/sys_sparc_64.c
index ccc88926bc00..5ad0494df367 100644
--- a/arch/sparc/kernel/sys_sparc_64.c
+++ b/arch/sparc/kernel/sys_sparc_64.c
@@ -340,21 +340,21 @@ SYSCALL_DEFINE6(sparc_ipc, unsigned int, call, int, first, unsigned long, second
if (call <= SEMTIMEDOP) {
switch (call) {
case SEMOP:
- err = sys_semtimedop(first, ptr,
- (unsigned int)second, NULL);
+ err = ksys_semtimedop(first, ptr,
+ (unsigned int)second, NULL);
goto out;
case SEMTIMEDOP:
- err = sys_semtimedop(first, ptr, (unsigned int)second,
+ err = ksys_semtimedop(first, ptr, (unsigned int)second,
(const struct __kernel_timespec __user *)
- (unsigned long) fifth);
+ (unsigned long) fifth);
goto out;
case SEMGET:
- err = sys_semget(first, (int)second, (int)third);
+ err = ksys_semget(first, (int)second, (int)third);
goto out;
case SEMCTL: {
- err = sys_semctl(first, second,
- (int)third | IPC_64,
- (unsigned long) ptr);
+ err = ksys_old_semctl(first, second,
+ (int)third | IPC_64,
+ (unsigned long) ptr);
goto out;
}
default:
@@ -365,18 +365,18 @@ SYSCALL_DEFINE6(sparc_ipc, unsigned int, call, int, first, unsigned long, second
if (call <= MSGCTL) {
switch (call) {
case MSGSND:
- err = sys_msgsnd(first, ptr, (size_t)second,
+ err = ksys_msgsnd(first, ptr, (size_t)second,
(int)third);
goto out;
case MSGRCV:
- err = sys_msgrcv(first, ptr, (size_t)second, fifth,
+ err = ksys_msgrcv(first, ptr, (size_t)second, fifth,
(int)third);
goto out;
case MSGGET:
- err = sys_msgget((key_t)first, (int)second);
+ err = ksys_msgget((key_t)first, (int)second);
goto out;
case MSGCTL:
- err = sys_msgctl(first, (int)second | IPC_64, ptr);
+ err = ksys_old_msgctl(first, (int)second | IPC_64, ptr);
goto out;
default:
err = -ENOSYS;
@@ -396,13 +396,13 @@ SYSCALL_DEFINE6(sparc_ipc, unsigned int, call, int, first, unsigned long, second
goto out;
}
case SHMDT:
- err = sys_shmdt(ptr);
+ err = ksys_shmdt(ptr);
goto out;
case SHMGET:
- err = sys_shmget(first, (size_t)second, (int)third);
+ err = ksys_shmget(first, (size_t)second, (int)third);
goto out;
case SHMCTL:
- err = sys_shmctl(first, (int)second | IPC_64, ptr);
+ err = ksys_old_shmctl(first, (int)second | IPC_64, ptr);
goto out;
default:
err = -ENOSYS;
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 88145da7d140..f7c561c4dcdd 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -1402,4 +1402,23 @@ static inline unsigned int ksys_personality(unsigned int personality)
return old;
}

+/* for __ARCH_WANT_SYS_IPC */
+long ksys_semtimedop(int semid, struct sembuf __user *tsops,
+ unsigned int nsops,
+ const struct __kernel_timespec __user *timeout);
+long ksys_semget(key_t key, int nsems, int semflg);
+long ksys_old_semctl(int semid, int semnum, int cmd, unsigned long arg);
+long ksys_msgget(key_t key, int msgflg);
+long ksys_old_msgctl(int msqid, int cmd, struct msqid_ds __user *buf);
+long ksys_msgrcv(int msqid, struct msgbuf __user *msgp, size_t msgsz,
+ long msgtyp, int msgflg);
+long ksys_msgsnd(int msqid, struct msgbuf __user *msgp, size_t msgsz,
+ int msgflg);
+long ksys_shmget(key_t key, size_t size, int shmflg);
+long ksys_shmdt(char __user *shmaddr);
+long ksys_old_shmctl(int shmid, int cmd, struct shmid_ds __user *buf);
+long compat_ksys_semtimedop(int semid, struct sembuf __user *tsems,
+ unsigned int nsops,
+ const struct old_timespec32 __user *timeout);
+
#endif
diff --git a/ipc/util.h b/ipc/util.h
index 0fcf8e719b76..5766c61aed0e 100644
--- a/ipc/util.h
+++ b/ipc/util.h
@@ -276,29 +276,7 @@ static inline int compat_ipc_parse_version(int *cmd)
*cmd &= ~IPC_64;
return version;
}
-#endif

-/* for __ARCH_WANT_SYS_IPC */
-long ksys_semtimedop(int semid, struct sembuf __user *tsops,
- unsigned int nsops,
- const struct __kernel_timespec __user *timeout);
-long ksys_semget(key_t key, int nsems, int semflg);
-long ksys_old_semctl(int semid, int semnum, int cmd, unsigned long arg);
-long ksys_msgget(key_t key, int msgflg);
-long ksys_old_msgctl(int msqid, int cmd, struct msqid_ds __user *buf);
-long ksys_msgrcv(int msqid, struct msgbuf __user *msgp, size_t msgsz,
- long msgtyp, int msgflg);
-long ksys_msgsnd(int msqid, struct msgbuf __user *msgp, size_t msgsz,
- int msgflg);
-long ksys_shmget(key_t key, size_t size, int shmflg);
-long ksys_shmdt(char __user *shmaddr);
-long ksys_old_shmctl(int shmid, int cmd, struct shmid_ds __user *buf);
-
-/* for CONFIG_ARCH_WANT_OLD_COMPAT_IPC */
-long compat_ksys_semtimedop(int semid, struct sembuf __user *tsems,
- unsigned int nsops,
- const struct old_timespec32 __user *timeout);
-#ifdef CONFIG_COMPAT
long compat_ksys_old_semctl(int semid, int semnum, int cmd, int arg);
long compat_ksys_old_msgctl(int msqid, int cmd, void __user *uptr);
long compat_ksys_msgrcv(int msqid, compat_uptr_t msgp, compat_ssize_t msgsz,
@@ -306,6 +284,7 @@ long compat_ksys_msgrcv(int msqid, compat_uptr_t msgp, compat_ssize_t msgsz,
long compat_ksys_msgsnd(int msqid, compat_uptr_t msgp,
compat_ssize_t msgsz, int msgflg);
long compat_ksys_old_shmctl(int shmid, int cmd, void __user *uptr);
-#endif /* CONFIG_COMPAT */
+
+#endif

#endif
--
2.20.0

2019-09-06 01:25:41

by Matt Turner

[permalink] [raw]
Subject: Re: [PATCH 2/2] ipc: fix sparc64 ipc() wrapper

On Thu, Sep 5, 2019 at 8:23 AM Arnd Bergmann <[email protected]> wrote:
>
> Matt bisected a sparc64 specific issue with semctl, shmctl and msgctl
> to a commit from my y2038 series in linux-5.1, as I missed the custom
> sys_ipc() wrapper that sparc64 uses in place of the generic version that
> I patched.
>
> The problem is that the sys_{sem,shm,msg}ctl() functions in the kernel
> now do not allow being called with the IPC_64 flag any more, resulting
> in a -EINVAL error when they don't recognize the command.
>
> Instead, the correct way to do this now is to call the internal
> ksys_old_{sem,shm,msg}ctl() functions to select the API version.
>
> As we generally move towards these functions anyway, change all of
> sparc_ipc() to consistently use those in place of the sys_*() versions,
> and move the required ksys_*() declarations into linux/syscalls.h
>
> Reported-by: Matt Turner <[email protected]>
> Fixes: 275f22148e87 ("ipc: rename old-style shmctl/semctl/msgctl syscalls")
> Cc: [email protected]
> Signed-off-by: Arnd Bergmann <[email protected]>
> ---
> Hi Matt,
>
> Can you check that this solves your problem?

Works great. Thank you Arnd!

Tested-by: Matt Turner <[email protected]>

2019-09-06 06:23:52

by Anatoly Pugachev

[permalink] [raw]
Subject: Re: [PATCH 2/2] ipc: fix sparc64 ipc() wrapper

On Thu, Sep 5, 2019 at 9:39 PM Arnd Bergmann <[email protected]> wrote:
>
> Matt bisected a sparc64 specific issue with semctl, shmctl and msgctl
> to a commit from my y2038 series in linux-5.1, as I missed the custom
> sys_ipc() wrapper that sparc64 uses in place of the generic version that
> I patched.
>
> The problem is that the sys_{sem,shm,msg}ctl() functions in the kernel
> now do not allow being called with the IPC_64 flag any more, resulting
> in a -EINVAL error when they don't recognize the command.
>
> Instead, the correct way to do this now is to call the internal
> ksys_old_{sem,shm,msg}ctl() functions to select the API version.
>
> As we generally move towards these functions anyway, change all of
> sparc_ipc() to consistently use those in place of the sys_*() versions,
> and move the required ksys_*() declarations into linux/syscalls.h
>
> Reported-by: Matt Turner <[email protected]>
> Fixes: 275f22148e87 ("ipc: rename old-style shmctl/semctl/msgctl syscalls")
> Cc: [email protected]
> Signed-off-by: Arnd Bergmann <[email protected]>


Not Matt,

but this patch fixes util-linux.git ipcs test-suite (make check)
regression for me on current sparc64 git kernel (5.3.0-rc7), which was
broken somewhere in between 4.19 (debian unstable kernel) and 5.3-rcX.

Thanks!

PS: wanted to bisect kernel, but Matt did it first.

2019-09-09 03:56:06

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 2/2] ipc: fix sparc64 ipc() wrapper

On Thu, Sep 5, 2019 at 5:24 PM Arnd Bergmann <[email protected]> wrote:

> diff --git a/arch/sparc/kernel/sys_sparc_64.c b/arch/sparc/kernel/sys_sparc_64.c
> index ccc88926bc00..5ad0494df367 100644
> --- a/arch/sparc/kernel/sys_sparc_64.c
> +++ b/arch/sparc/kernel/sys_sparc_64.c
> @@ -340,21 +340,21 @@ SYSCALL_DEFINE6(sparc_ipc, unsigned int, call, int, first, unsigned long, second
> if (call <= SEMTIMEDOP) {
> switch (call) {
> case SEMOP:
> - err = sys_semtimedop(first, ptr,
> - (unsigned int)second, NULL);
> + err = ksys_semtimedop(first, ptr,
> + (unsigned int)second, NULL);
> goto out;

The zero-day bot found a link error in sparc64 allnoconfig:

arch/sparc/kernel/sys_sparc_64.o: In function `__se_sys_sparc_ipc':
>> sys_sparc_64.c:(.text+0x724): undefined reference to `ksys_semtimedop'
>> sys_sparc_64.c:(.text+0x76c): undefined reference to `ksys_old_msgctl'
>> sys_sparc_64.c:(.text+0x7a8): undefined reference to `ksys_semget'
>> sys_sparc_64.c:(.text+0x7c8): undefined reference to `ksys_old_semctl'
>> sys_sparc_64.c:(.text+0x7e4): undefined reference to `ksys_msgsnd'
>> sys_sparc_64.c:(.text+0x7fc): undefined reference to `ksys_shmget'
>> sys_sparc_64.c:(.text+0x808): undefined reference to `ksys_shmdt'
sys_sparc_64.c:(.text+0x828): undefined reference to `ksys_semtimedop'
>> sys_sparc_64.c:(.text+0x844): undefined reference to `ksys_old_shmctl'
>> sys_sparc_64.c:(.text+0x858): undefined reference to `ksys_msgget'
>> sys_sparc_64.c:(.text+0x86c): undefined reference to `ksys_msgrcv'

I've added this hunk to my patch and plan to send both fixes to Linus
in the next few days, after I get a positive report from the bot as well:

--- a/arch/sparc/kernel/sys_sparc_64.c
+++ b/arch/sparc/kernel/sys_sparc_64.c
@@ -336,6 +336,9 @@ SYSCALL_DEFINE6(sparc_ipc, unsigned int, call,
int, first, unsigned long, second
{
long err;

+ if (!IS_ENABLED(CONFIG_SYSVIPC))
+ return -ENOSYS;
+
/* No need for backward compatibility. We can start fresh... */

if (call <= SEMTIMEDOP) {
switch (call) {