* [RFC PATCH 0/1] Fix handling semctl on x86-32 kernels @ 2022-05-15 20:01 Maxim Zhukov 2022-05-15 20:01 ` [RFC PATCH 1/1] ipc, sem: fix backward compatibility with " Maxim Zhukov 0 siblings, 1 reply; 5+ messages in thread From: Maxim Zhukov @ 2022-05-15 20:01 UTC (permalink / raw) To: chi.minghao, varad.gautam, arnd Cc: akpm, shakeelb, vasily.averin, manfred, dbueso, linux-kernel, Maxim Zhukov I'm not sure about this patch. Maybe there is a better way to solve this problem: all libc sends with cmd IPC_64 flag[1][2][3], but x86-32 kernel does not have compat syscall layer to handle correctly semctl command. [1]: https://elixir.bootlin.com/uclibc-ng/v1.0.40/source/libc/misc/sysvipc/sem.c#L58 [2]: https://elixir.bootlin.com/musl/v1.2.3/source/src/ipc/semctl.c#L48 -> https://elixir.bootlin.com/musl/v1.2.3/source/src/ipc/ipc.h#L22 [3]: https://elixir.bootlin.com/glibc/glibc-2.35/source/sysdeps/unix/sysv/linux/semctl.c#L124 Maxim Zhukov (1): ipc, sem: fix backward compatibility with x86-32 kernels ipc/sem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 2.36.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [RFC PATCH 1/1] ipc, sem: fix backward compatibility with x86-32 kernels 2022-05-15 20:01 [RFC PATCH 0/1] Fix handling semctl on x86-32 kernels Maxim Zhukov @ 2022-05-15 20:01 ` Maxim Zhukov 2022-05-16 21:06 ` Andrew Morton 2022-05-16 22:00 ` Arnd Bergmann 0 siblings, 2 replies; 5+ messages in thread From: Maxim Zhukov @ 2022-05-15 20:01 UTC (permalink / raw) To: chi.minghao, varad.gautam, arnd Cc: akpm, shakeelb, vasily.averin, manfred, dbueso, linux-kernel, Maxim Zhukov Since with commit 275f22148e87 ("ipc: rename old-style shmctl/semctl/msgctl syscalls") we have changed behavior: ksys_semctl lost parse ipc version (ipc_parse_version), because the new syscall works with IPC_64 only, but the parse function has some side effect - it removes IPC_64 bit from `cmd`. Some libc forced sends IPC_64 bit in semctl syscall[1][2][3], this leads to a bug - X86-32 kernel does not have compat headers and does not correctly parse received command (cmd) from semctl syscall: cmd have actual command and IPC_64 bit, thus throw EINVAL error in ksys_semctl This commit forcibly removes IPC_64 bit from the cmd for restore backward compatibility. [1]: https://elixir.bootlin.com/uclibc-ng/v1.0.40/source/libc/misc/sysvipc/sem.c#L58 [2]: https://elixir.bootlin.com/musl/v1.2.3/source/src/ipc/semctl.c#L48 -> https://elixir.bootlin.com/musl/v1.2.3/source/src/ipc/ipc.h#L22 [3]: https://elixir.bootlin.com/glibc/glibc-2.35/source/sysdeps/unix/sysv/linux/semctl.c#L124 Signed-off-by: Maxim Zhukov <mussitantesmortem@gmail.com> --- ipc/sem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ipc/sem.c b/ipc/sem.c index 0dbdb98fdf2d..824244170000 100644 --- a/ipc/sem.c +++ b/ipc/sem.c @@ -1706,7 +1706,7 @@ static long ksys_semctl(int semid, int semnum, int cmd, unsigned long arg, int v SYSCALL_DEFINE4(semctl, int, semid, int, semnum, int, cmd, unsigned long, arg) { - return ksys_semctl(semid, semnum, cmd, arg, IPC_64); + return ksys_semctl(semid, semnum, cmd & (~IPC_64), arg, IPC_64); } #ifdef CONFIG_ARCH_WANT_IPC_PARSE_VERSION -- 2.36.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [RFC PATCH 1/1] ipc, sem: fix backward compatibility with x86-32 kernels 2022-05-15 20:01 ` [RFC PATCH 1/1] ipc, sem: fix backward compatibility with " Maxim Zhukov @ 2022-05-16 21:06 ` Andrew Morton 2022-05-16 22:07 ` Arnd Bergmann 2022-05-16 22:00 ` Arnd Bergmann 1 sibling, 1 reply; 5+ messages in thread From: Andrew Morton @ 2022-05-16 21:06 UTC (permalink / raw) To: Maxim Zhukov Cc: chi.minghao, varad.gautam, arnd, shakeelb, vasily.averin, manfred, dbueso, linux-kernel, Maxim Zhukov On Sun, 15 May 2022 23:01:03 +0300 Maxim Zhukov <crazycdeveloper@gmail.com> wrote: > Since with commit 275f22148e87 ("ipc: rename old-style shmctl/semctl/msgctl syscalls") > we have changed behavior: > > ksys_semctl lost parse ipc version (ipc_parse_version), because the > new syscall works with IPC_64 only, but the parse function has some > side effect - it removes IPC_64 bit from `cmd`. > > Some libc forced sends IPC_64 bit in semctl syscall[1][2][3], this leads to > a bug - X86-32 kernel does not have compat headers and does not > correctly parse received command (cmd) from semctl syscall: cmd have actual > command and IPC_64 bit, thus throw EINVAL error in ksys_semctl > > This commit forcibly removes IPC_64 bit from the cmd for restore > backward compatibility. > > [1]: https://elixir.bootlin.com/uclibc-ng/v1.0.40/source/libc/misc/sysvipc/sem.c#L58 > [2]: https://elixir.bootlin.com/musl/v1.2.3/source/src/ipc/semctl.c#L48 -> https://elixir.bootlin.com/musl/v1.2.3/source/src/ipc/ipc.h#L22 > [3]: https://elixir.bootlin.com/glibc/glibc-2.35/source/sysdeps/unix/sysv/linux/semctl.c#L124 > Thanks. 275f22148e87 was three years ago. Can you suggest why it took so long for this to be discovered? ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC PATCH 1/1] ipc, sem: fix backward compatibility with x86-32 kernels 2022-05-16 21:06 ` Andrew Morton @ 2022-05-16 22:07 ` Arnd Bergmann 0 siblings, 0 replies; 5+ messages in thread From: Arnd Bergmann @ 2022-05-16 22:07 UTC (permalink / raw) To: Andrew Morton Cc: Maxim Zhukov, chi.minghao, varad.gautam, Arnd Bergmann, Shakeel Butt, vasily.averin, Manfred Spraul, Davidlohr Bueso, Linux Kernel Mailing List, Maxim Zhukov On Mon, May 16, 2022 at 10:06 PM Andrew Morton <akpm@linux-foundation.org> wrote: > > On Sun, 15 May 2022 23:01:03 +0300 Maxim Zhukov <crazycdeveloper@gmail.com> wrote: > > > Since with commit 275f22148e87 ("ipc: rename old-style shmctl/semctl/msgctl syscalls") > > we have changed behavior: > > > > ksys_semctl lost parse ipc version (ipc_parse_version), because the > > new syscall works with IPC_64 only, but the parse function has some > > side effect - it removes IPC_64 bit from `cmd`. > > > > Some libc forced sends IPC_64 bit in semctl syscall[1][2][3], this leads to > > a bug - X86-32 kernel does not have compat headers and does not > > correctly parse received command (cmd) from semctl syscall: cmd have actual > > command and IPC_64 bit, thus throw EINVAL error in ksys_semctl > > > > This commit forcibly removes IPC_64 bit from the cmd for restore > > backward compatibility. > > > > [1]: https://elixir.bootlin.com/uclibc-ng/v1.0.40/source/libc/misc/sysvipc/sem.c#L58 > > [2]: https://elixir.bootlin.com/musl/v1.2.3/source/src/ipc/semctl.c#L48 -> https://elixir.bootlin.com/musl/v1.2.3/source/src/ipc/ipc.h#L22 > > [3]: https://elixir.bootlin.com/glibc/glibc-2.35/source/sysdeps/unix/sysv/linux/semctl.c#L124 > > > > Thanks. > > 275f22148e87 was three years ago. Can you suggest why it took so long > for this to be discovered? I think it only shows up with a uclibc-ng built against 32-bit kernel headers from linux-5.1 or new for m68k, mips-o32, powerpc, s390, sh, sparc, and x86-32 (list is from my original commit 275f22148e87), not for the more popular musl or glibc libraries. If sysvipc is used this rarely on uclibc-ng, maybe we can fix it by making it behave the same way as glibc instead? I agree the kernel interface is easy to get wrong here because of the subtle difference between ipc() and semctl(), but this was an intentional design choice at the time, and it did not affect the behavior of the existing syscalls, only the newly added calls. Arnd ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC PATCH 1/1] ipc, sem: fix backward compatibility with x86-32 kernels 2022-05-15 20:01 ` [RFC PATCH 1/1] ipc, sem: fix backward compatibility with " Maxim Zhukov 2022-05-16 21:06 ` Andrew Morton @ 2022-05-16 22:00 ` Arnd Bergmann 1 sibling, 0 replies; 5+ messages in thread From: Arnd Bergmann @ 2022-05-16 22:00 UTC (permalink / raw) To: Maxim Zhukov Cc: chi.minghao, varad.gautam, Arnd Bergmann, Andrew Morton, Shakeel Butt, vasily.averin, Manfred Spraul, Davidlohr Bueso, Linux Kernel Mailing List, Maxim Zhukov On Sun, May 15, 2022 at 9:01 PM Maxim Zhukov <crazycdeveloper@gmail.com> wrote: > > Since with commit 275f22148e87 ("ipc: rename old-style shmctl/semctl/msgctl syscalls") > we have changed behavior: > > ksys_semctl lost parse ipc version (ipc_parse_version), because the > new syscall works with IPC_64 only, but the parse function has some > side effect - it removes IPC_64 bit from `cmd`. > > Some libc forced sends IPC_64 bit in semctl syscall[1][2][3], this leads to > a bug - X86-32 kernel does not have compat headers and does not > correctly parse received command (cmd) from semctl syscall: cmd have actual > command and IPC_64 bit, thus throw EINVAL error in ksys_semctl That is unfortunate, and clearly against the intention of my commit from back then: the idea was that any libc that moves from the old to the new syscalls would drop support for the ancient IPC version and no longer have to pass the IPC_64 flag. Given how long it took to run into this bug, let's try to figure out exactly what options we have to address this before applying any patch. > This commit forcibly removes IPC_64 bit from the cmd for restore > backward compatibility. > > [1]: https://elixir.bootlin.com/uclibc-ng/v1.0.40/source/libc/misc/sysvipc/sem.c#L58 > [2]: https://elixir.bootlin.com/musl/v1.2.3/source/src/ipc/semctl.c#L48 -> https://elixir.bootlin.com/musl/v1.2.3/source/src/ipc/ipc.h#L22 > [3]: https://elixir.bootlin.com/glibc/glibc-2.35/source/sysdeps/unix/sysv/linux/semctl.c#L124 I think musl handles it correctly here: it always calls the old-style ipc() syscall if that is available. For glibc, I'm not completely sure but I think that also does it correctly, defining IPC_64 to 0 for the configuration in which it calls sys_semctl(). The uclibc-ng implementation is clearly wrong here, I assume that's what you tested with? While your patch would make uclibc-ng work on the affected architectures and not break the other libc implementations, it is still an ABI change to allow the 0x100 bit to be set in the "cmd" value. This is different from both the traditional behavior on the ipc() syscall, and from the traditional behavior on architectures that had semctl() without IPC_64. > Signed-off-by: Maxim Zhukov <mussitantesmortem@gmail.com> > --- > ipc/sem.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/ipc/sem.c b/ipc/sem.c > index 0dbdb98fdf2d..824244170000 100644 > --- a/ipc/sem.c > +++ b/ipc/sem.c > @@ -1706,7 +1706,7 @@ static long ksys_semctl(int semid, int semnum, int cmd, unsigned long arg, int v > > SYSCALL_DEFINE4(semctl, int, semid, int, semnum, int, cmd, unsigned long, arg) > { > - return ksys_semctl(semid, semnum, cmd, arg, IPC_64); > + return ksys_semctl(semid, semnum, cmd & (~IPC_64), arg, IPC_64); > } I don't think it makes sense to do this for semctl but not also for shmctl and msqctl -- whatever we end up doing should be the same across all three. Arnd ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-05-16 22:08 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-05-15 20:01 [RFC PATCH 0/1] Fix handling semctl on x86-32 kernels Maxim Zhukov 2022-05-15 20:01 ` [RFC PATCH 1/1] ipc, sem: fix backward compatibility with " Maxim Zhukov 2022-05-16 21:06 ` Andrew Morton 2022-05-16 22:07 ` Arnd Bergmann 2022-05-16 22:00 ` Arnd Bergmann
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).