Bug 112271

Summary:	SysV semaphore sempid should not be set by call to semctl()
Product:	Other	Reporter:	Philip Semanchuk (linux_kernel.20.ick)
Component:	Other	Assignee:	other_other
Status:	NEW ---
Severity:	low	CC:	manfred, mtk.manpages
Priority:	P1
Hardware:	All
OS:	Linux
Kernel Version:	3.2.0-23-generic	Subsystem:
Regression:	No	Bisected commit-id:

Description Philip Semanchuk 2016-02-10 17:11:29 UTC

I’ve found a place where the kernel slightly violates the POSIX specification. It also disagrees with its own documentation. Specifically, the kernel correctly sets a SysV semaphore’s sempid attribute when performing an “operation” (i.e. a call to semop()), but it also sets sempid when setting the semaphore’s value (a call to semctl(SETVAL)). I believe setting sempid for any call other than semop() is in disagreement with the specification & doc and therefore a bug.

This is 100% re-createable. It’s how the code was designed to work, I just don't think it agrees with the spec.

Details —
SysV IPC semaphores have a data structure that includes a member called sempid. The source code comment says it’s the “pid of last operation”:
https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/tree/ipc/sem.c?id=388f7b1d6e8ca06762e2454d28d6c3c55ad0fe95#n95

I argue that “operation” implies a call to semop(), and only semop(), not other calls.

sempid is set when the semaphore’s value is set via semctl(); this is the part I think is incorrect:
https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/tree/ipc/sem.c?id=388f7b1d6e8ca06762e2454d28d6c3c55ad0fe95#n1328

semctl() is also used to change other semaphore attributes (e.g. the mode). sempid is only set when the cmd parameter to semctl() is SETVAL.

The OpenGroup doc for semop() specifically states that it sets sempid:
http://pubs.opengroup.org/onlinepubs/7908799/xsh/semop.html

While the equivalent doc for semctl() does not mention sempid, implying that it should not set sempid:
http://pubs.opengroup.org/onlinepubs/7908799/xsh/semctl.html

These man pages state that sempid is the “ID of process that did last op”:
http://man7.org/linux/man-pages/man2/semop.2.html
http://man7.org/linux/man-pages/man7/svipc.7.html

This man page specifically states that sempid is “the PID of the process that executed the last semop(2) call”:
http://man7.org/linux/man-pages/man2/semctl.2.html

IMO, FreeBSD behaves correctly. FreeBSD does not set sempid when setting the value:
https://github.com/freebsd/freebsd/blob/master/sys/kern/sysv_sem.c#L546

FreeBSD *does* set sempid when performing an operation:
https://github.com/freebsd/freebsd/blob/master/sys/kern/sysv_sem.c#L1262

Sorry I don’t have a simple C code example. I discovered this while testing a Python package that I wrote (sysv_ipc) that wraps the SysV IPC API. The package is mature & stable. I can demonstrate the behavior easily through that package, as in the examples below. Note that my C implementation of the sysv_ipc.Semaphore() constructor calls semctl(SETVAL) before returning which triggers setting sempid on Linux. The source code for my sysv_ipc Semaphore init is here:
https://bitbucket.org/philip_semanchuk/sysv_ipc/src/0cd9d5138d7388689981e754aa8df35e30eab0c4/semaphore.c?fileviewer=file-view-default#semaphore.c-458

= Example on PCBSD 10.2 x64 = 

% python3.4
Python 3.4.4 (default, Jan  9 2016, 14:57:02) 
[GCC 4.2.1 Compatible FreeBSD Clang 3.4.1 (tags/RELEASE_34/dot1-final 208032)] on freebsd10
Type "help", "copyright", "credits" or "license" for more information.
>>> import sysv_ipc
>>> import os
>>> os.getpid()
11239
>>> sem = sysv_ipc.Semaphore(None, sysv_ipc.IPC_CREX) # calls semget() and
>>> semctl(SETVAL)
>>> sem.last_pid # returns semctl(GETPID)
0
>>> sem.release() # calls semop()
>>> sem.last_pid
11239
>>> 

= Example on Linux Mint 17 x64 = 
$ python3
Python 3.2.3 (default, Jun 18 2015, 21:46:58) 
[GCC 4.6.3] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import sysv_ipc
>>> import os
>>> os.getpid()
1937
>>> sem = sysv_ipc.Semaphore(None, sysv_ipc.IPC_CREX) # calls semget() and
>>> semctl(SETVAL)
>>> sem.last_pid # returns semctl(GETPID)
1937
>>> sem.release() # calls semop()
>>> sem.last_pid
1937
>>> 

This is the version against which I tested:
$ cat /proc/version
Linux version 3.2.0-23-generic (buildd@crested) (gcc version 4.6.3 (Ubuntu/Linaro 4.6.3-1ubuntu4) ) #36-Ubuntu SMP Tue Apr 10 20:39:51 UTC 2012

However the relevant portion of sem.c hasn’t changed since Linus’ initial commit of Linux-2.6.12-rc2 to git:
https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/tree/ipc/sem.c?id=1da177e4c3f41524e886b7f1b8a0c1fc7321cac2

Comment 1 Manfred Spraul 2016-02-26 20:22:11 UTC

The code was probably like this since 0.99.
And: At least Opensolaris and Apple Darwin also have the same behavior.

Thus I'm a bit reluctant to change the Linux behavior, it may break applications.

Comment 2 Philip Semanchuk 2016-02-27 17:58:14 UTC

I can confirm that Darwin/OS X behaves the same. (I’m on a Mac so it’s easy to test.) Apple’s SysV and POSIX IPC implementations are not typically held up as exemplars, but I agree it’s significant here that Darwin is one of several systems that behaves like this.

I wonder how AIX and HP-UX behave?

It seems like all the BSDs (Open/Net/Free and derivatives) behave correctly. I’ve only tested FreeBSD, but it’s pretty easy to spot in the source code where the others set sempid.

AFAICT FreeBSD never had this bug. The oldest version of the relevant FreeBSD file (sysv_sem.c) that I can find is from 1994 when SysV IPC support was added. The commit comment says “Added SYSV ipcs. Obtained from: NetBSD and FreeBSD-1.1.5”:
https://github.com/freebsd/freebsd/commit/580fe46632ff236a0f268c29705d04d44ad2f89a

Darwin is supposedly based on a lot of code from BSD but apparently Apple didn’t copy their SysV implementation, or they copied it and then modified it.

So Linux, OpenSolaris, and Darwin have the bug.

The BSDs do not.

We don’t know about AIX and HP-UX, and I don’t have a way to test them.

I’m ambivalent about whether or not it should be fixed. I agree that fixing the bug may break existing apps, but not fixing it makes code non-portable across Linux & BSD. In either case, the impact is probably small because I don’t think this feature of SysV IPC is used a lot.

If the Linux code doesn’t change, the man pages that I referenced (for semop, svipc, and semctl) need an update.

Comment 3 Michael Kerrisk 2016-02-28 19:15:34 UTC

(In the Feb 2016 mail thread that tried to modify the Linux behavior, "[PATCH] Don't set sempid in semctl syscall", Manfred Spraul added a pointer to a useful page he created long ago. Adding the URL here for reference: http://calculix-rpm.sourceforge.net/sysvsem.html)

So, given that there is implementation variation that probably predates POSIX.1 (I'm assuming that the OpenSolaris behavior has an ancestry that stretches way back), I'd argue that the fault here lies with POSIX, inasmuch as it failed to capture the full variation in existing implementation behavior. (The BSD implementations of System V IPC were post facto.) Generally POSIX.1 does not try to prescribe away existing implementation behavior, but instead creates a loose spec, not that an implementation "may do such and such".

I've added the following text to the semctl(2) man page:

   The sempid value
       POSIX.1 defines sempid as the "process ID of [the] last  opera‐
       tion"  on  a semaphore, and explicitly notes that this value is
       set by a successful semop(2) call, with the implication that no
       other interface affects the sempid value.

       While some implementations conform to the behavior specified in
       POSIX.1, others do not.  (The fault  here  probably  lies  with
       POSIX.1  inasmuch as it likely failed to capture the full range
       of existing implementation behaviors.)  Various other implemen‐
       tations also update sempid for the other operations that update
       the value of a semaphore: the SETVAL and SETALL operations,  as
       well as the semaphore adjustments performed on process termina‐
       tion as a consequence of the use  of  the  SEM_UNDO  flag  (see
       semop(2)).

       Linux  also  updates sempid for SETVAL operations and semaphore
       adjustments.  However, somewhat  inconsistently,  it  does  not
       update sempid for SETALL operations.  While the SETALL behavior
       might be viewed as a bug, the behavior is longstanding, and  is
       probably unlikely to change.

Comment 4 Philip Semanchuk 2016-03-02 21:34:14 UTC

These man pages might need a tweak too, since they explicitly state that sempid is the “ID of process that did last op”:
http://man7.org/linux/man-pages/man2/semop.2.html
http://man7.org/linux/man-pages/man7/svipc.7.html

Comment 5 Michael Kerrisk 2016-03-03 07:56:20 UTC

Thanks, Phil. Done.(In reply to Philip Semanchuk from comment #4)
> These man pages might need a tweak too, since they explicitly state that
> sempid is the “ID of process that did last op”:
> http://man7.org/linux/man-pages/man2/semop.2.html
> http://man7.org/linux/man-pages/man7/svipc.7.html

Thanks. Tweaked!