kern/123685: p1003_1b.nsems can become negative/semaphore counter incorrect

[Mike van der Schaar]

>Number:         123685
>Category:       kern
>Synopsis:       p1003_1b.nsems can become negative/semaphore counter incorrect
>Confidential:   no
>Severity:       serious
>Priority:       low
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Wed May 14 22:10:02 UTC 2008
>Closed-Date:
>Last-Modified:
>Originator:     Mike van der Schaar
>Release:        FreeBSD 7.0-RELEASE i386
>Organization:
>Environment:
System: FreeBSD legendre.labnet 7.0-RELEASE FreeBSD 7.0-RELEASE #1: Wed May 14 15:39:42 CEST 2008 root at legendre.labnet:/usr/obj/usr/src/sys/LEGENDRE i386



>Description:
When a posix sempahore is created, but the number of semaphores is already at its maximum, it is immediately freed again in uipc_sem.c:sem_create():229. The nsems counter is not updated before freeing the semaphore. But in uipc_sem.c:sem_free():451 the semaphore counter is always decreased.

This effectively allows the creation of more than 30 semaphores, while the counter stays at 30. When semaphores are then freed at a later point the counter will become negative.

>How-To-Repeat:
The program just forces more than 30 semaphores to be created. After running the semaphore counter should be at -5 (10 too many, only half of those are allowed).

#include <stdlib.h>
#include <stdio.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <fcntl.h>
#include <string.h>
#include <semaphore.h>

int main(void)
{
    int i;
    char name[16];
    sem_t *sem[40];

    memset( sem, 0, sizeof(sem_t *)*40 );
    for ( i = 0; i < 40; i++ )
    {
        sprintf( name, "/sem_%.3d", i );
        sem[i] = sem_open( name,  O_CREAT|O_EXCL, S_IRUSR|S_IWUSR, 1 );
    }
    for ( i = 0; i < 40; i++ )
    {
        if ( sem[i] )
            sem_close( sem[i] );
        sprintf( name, "/sem_%.3d", i );
        sem_unlink( name );
    }

    return 0;
}

After executing :
(legendre:~) mike> sysctl p1003_1b
p1003_1b.asynchronous_io: 0
p1003_1b.mapped_files: 1
p1003_1b.memlock: 0
p1003_1b.memlock_range: 0
p1003_1b.memory_protection: 0
p1003_1b.message_passing: 200112
p1003_1b.prioritized_io: 0
p1003_1b.priority_scheduling: 1
p1003_1b.realtime_signals: 200112
p1003_1b.semaphores: 0
p1003_1b.fsync: 0
p1003_1b.shared_memory_objects: 1
p1003_1b.synchronized_io: 0
p1003_1b.timers: 200112
p1003_1b.aio_listio_max: -1
p1003_1b.aio_max: -1
p1003_1b.aio_prio_delta_max: -1
p1003_1b.delaytimer_max: 2147483647
p1003_1b.mq_open_max: 0
p1003_1b.pagesize: 4096
p1003_1b.rtsig_max: 62
p1003_1b.sem_nsems_max: 30
p1003_1b.sem_value_max: -1
p1003_1b.sigqueue_max: 128
p1003_1b.timer_max: 32
p1003_1b.nsems: -5

>Fix:

One solution would be to increase the counter directly after the semaphore is created and added to the list. If the check for the maximum number of semaphores can be moved up before creating and adding the semaphore, without affecting efficiency too much because of the extra mtx_lock, then that might be better.
>Release-Note:
>Audit-Trail:
>Unformatted: