Bug 173724 - [kernel] SYSV semaphore adjustment incorrect for SETVAL for array with 2 or more semaphores
Summary: [kernel] SYSV semaphore adjustment incorrect for SETVAL for array with 2 or m...
Status: Open
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 12.2-RELEASE
Hardware: Any Any
: Normal Affects Only Me
Assignee: freebsd-bugs (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-11-19 17:00 UTC by Anton Lavrentiev
Modified: 2021-02-23 07:34 UTC (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Anton Lavrentiev 2012-11-19 17:00:00 UTC
SYSV semaphore adjustment is not done correctly for arrays with more than one semathore.


#include <errno.h>
#include <stdio.h>
#include <string.h>
#include <sys/ipc.h>
#include <sys/sem.h>


#define BUG1
#define BUG2


#define SEMKEY 1


union semun {
    int val;                    /* value for SETVAL */
    struct semid_ds *buf;       /* buffer for IPC_STAT, IPC_SET */
    unsigned short  *array;     /* array for GETALL, SETALL */
};


static void doCriticalWork(void)
{
    return;
}


int main(void)
{
    struct sembuf lock[2];
    int n, semid;

    if ((semid = semget(SEMKEY, 2, IPC_CREAT | 0666)) < 0) {
        perror("semget(IPC_CREATE)");
        return 1;
    }

#ifdef BUG1
    lock[0].sem_num = 0;
    lock[0].sem_op  = 0;
    lock[0].sem_flg = IPC_NOWAIT;
    lock[1].sem_num = 0;
    lock[1].sem_op  = 1;

#ifdef BUG2
    lock[1].sem_flg = SEM_UNDO;
#else
    lock[1].sem_flg = 0;
#endif

    if (semop(semid, lock, 2) != 0) {
        perror("semop(LOCK[0])");
        return 1;
    }
#endif

    for (n = 0; ; ++n) {
        static const union semun arg = { 0 };
        int error;

        printf("Iteration %d\n", n);

        lock[0].sem_num = 1;
        lock[0].sem_op  = 0; /* precondition:  [1] == 0 */
        lock[0].sem_flg = 0;
        lock[1].sem_num = 1;
        lock[1].sem_op  = 1; /* postcondition: [1] == 1 */
        lock[1].sem_flg = SEM_UNDO;

        if (semop(semid, lock, 2) < 0) {
            error = errno;
            fprintf(stderr, "semop(LOCK[1]): %d, %s\n",
                    error, strerror(error));
            break;
        }

        doCriticalWork();

#if 1
        if (semctl(semid, 1, SETVAL, arg) < 0) {
            error = errno;
            fprintf(stderr, "semctl(UNLOCK[1]): %d, %s\n",
                    error, strerror(error));
            break;
        }
#else
        lock[0].sem_num =  1;
        lock[0].sem_op  = -1;
        lock[0].sem_flg =  SEM_UNDO | IPC_NOWAIT;

        if (semop(semid, lock, 1) < 0) {
            error = errno;
            fprintf(stderr, "semop(UNLOCK[1]): %d, %s\n",
                    error, strerror(error));
            break;
        }
#endif

    }

    return 1;
}

Fix: 

The probable cause is the implementation semundo_clear() found in
sys/kern/sysv_sem.c .  FYI, originally the bug was found on CYGWIN, which
adopts the similar FreeBSD code to implement the semaphore arrays.
How-To-Repeat: Compile and run the code shown in the problem description.  It will stop
at iteration 16384 suggesting the UNDO adjustment value overflow with
ERANGE (which should have not happened because SETVAL should have cleared
all the UNDO adjustments on each loop iteration).
Comment 1 Eitan Adler freebsd_committer freebsd_triage 2017-12-31 08:00:14 UTC
For bugs matching the following criteria:

Status: In Progress Changed: (is less than) 2014-06-01

Reset to default assignee and clear in-progress tags.

Mail being skipped
Comment 2 Anton Lavrentiev 2021-02-23 07:14:46 UTC
This is still an issue even though it's version 12.2 now...  Nothing fixed.

...
Iteration 16382
Iteration 16383
Iteration 16384
semop(LOCK[1]): 34, Result too large

> uname -a
FreeBSD bsdtest64.be-md.ncbi.nlm.nih.gov 12.2-RELEASE-p3 FreeBSD 12.2-RELEASE-p3 GENERIC  amd64
Comment 3 Anton Lavrentiev 2021-02-23 07:34:54 UTC
The bug was fixed in Cygwin (long time ago!) by moving the following comparison with -1 for inequality _inside_ this "if" in the semundo_clear() function.  In the current FreeBSD source code it's still outside the if:

                                if (semnum == -1 || sunptr->un_num == semnum) {
                                ...
+					if (semnum != -1)
+						break;
 				}
-				if (semnum != -1)
-					break;