diff mbox series

[uclibc-ng-devel] sem: test semctl's command instead of masking its bits

Message ID 20260831080150.3550277-1-lordrasmus@gmail.com
State New
Headers show
Series [uclibc-ng-devel] sem: test semctl's command instead of masking its bits | expand

Commit Message

Ramin Moussavi Aug. 31, 2026, 8:01 a.m. UTC 
From: ramin <lordrasmus@gmail.com>

The time64 fixup for sem_otime/sem_ctime was guarded with

    if ((cmd & (IPC_STAT | IPC_SET)) && (arg.__pad != NULL))

IPC_SET is 1 and IPC_STAT is 2, so the mask is 3: the condition asks
whether either of the low two bits of cmd is set, not whether cmd is one of
the two.  It is therefore also true for IPC_INFO (3), GETPID (11),
GETALL (13), GETNCNT (14), GETZCNT (15), SETALL (17) and SEM_INFO (19).
GETVAL (12) and SETVAL (16) escape only because their low bits happen to be
zero.

For those commands the fourth argument is not a struct semid_ds.  In the
32-bit time64 layout struct ipc_perm is 36 bytes and sem_otime/sem_ctime sit
at offsets 56 and 64 of the 80-byte struct semid_ds, so the two assignments
put 16 bytes at offset 56 of whatever was passed: an unsigned short array
for GETALL and SETALL, a 40-byte struct seminfo for the two INFO commands,
and for GETPID, GETNCNT and GETZCNT a pointer the caller never passed at
all, so whatever a leftover register holds.

msgctl() and shmctl() perform the same recombination but take a typed
pointer, so a caller cannot hand them another type.  semctl(), with its
varargs union semun, is the only one of the three that has to look at cmd.

IPC_SET is dropped rather than compared: there the kernel only reads the
buffer, and recomputing sem_otime/sem_ctime from halves it did not touch
changes nothing.  SEM_STAT is added, which the mask caught by accident and
which does return a struct semid_ds.

Confirmed by tst-semctl in uclibc-ng-test, extended for this: 14 CI targets
failed on it, in both byte orders -- on riscv32 elements 28..35 of a
40 semaphore array came back as 19..26, on m68k as 21,22,19,20,25,26,23,24,
the two words swapped -- and IPC_INFO wrote past its seminfo.  With this
change the test passes on i686 with UCLIBC_USE_TIME64.

Signed-off-by: Ramin Moussavi <lordrasmus@gmail.com>
---
 libc/misc/sysvipc/sem.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
diff mbox series

Patch

diff --git a/libc/misc/sysvipc/sem.c b/libc/misc/sysvipc/sem.c
index 0f93e5451..48f0a8491 100644
--- a/libc/misc/sysvipc/sem.c
+++ b/libc/misc/sysvipc/sem.c
@@ -58,10 +58,10 @@  int semctl(int semid, int semnum, int cmd, ...)
 #ifdef __NR_semctl
     int __ret = __semctl(semid, semnum, cmd | __IPC_64, arg.__pad);
 #if (__WORDSIZE == 32) && defined(__UCLIBC_USE_TIME64__)
-    // Only when cmd is IPC_STAT and IPC_SET, semun points to struct semid_ds.
-    // At this point, arg.__pad should not be NULL, but a check is added just
-    // to be safe.
-    if ((cmd & (IPC_STAT | IPC_SET)) && (arg.__pad != NULL)) {
+    /* IPC_STAT and SEM_STAT are the commands the kernel answers with a
+       struct semid_ds.  For every other one the fourth argument has a
+       different type, or the caller passed none at all.  */
+    if ((cmd == IPC_STAT || cmd == SEM_STAT) && arg.buf != NULL) {
         arg.buf->sem_otime = (__time_t)arg.buf->__sem_otime_internal_1 | (__time_t)(arg.buf->__sem_otime_internal_2) << 32;
         arg.buf->sem_ctime = (__time_t)arg.buf->__sem_ctime_internal_1 | (__time_t)(arg.buf->__sem_ctime_internal_2) << 32;
     }