Synopsis: System V Semaphore denial of service
NetBSD versions: NetBSD 1.4, 1.4.1, and 1.4.2
Thanks to: Bill Sommerfeld
Reported in NetBSD Security Advisory: SA2000-004


*** sys/kern/sysv_sem.c.orig	1998/10/21 22:24:28	1.32
--- sys/kern/sysv_sem.c	2000/05/28 23:46:44
***************
*** 21,27 ****
  #include <sys/syscallargs.h>
  
  int	semtot = 0;
- struct	proc *semlock_holder = NULL;
  
  #ifdef SEM_DEBUG
  #define SEM_PRINTF(a) printf a
--- 21,26 ----
***************
*** 29,39 ****
  #define SEM_PRINTF(a)
  #endif
  
- void semlock __P((struct proc *));
  struct sem_undo *semu_alloc __P((struct proc *));
  int semundo_adjust __P((struct proc *, struct sem_undo **, int, int, int));
  void semundo_clear __P((int, int));
  
  void
  seminit()
  {
--- 28,42 ----
  #define SEM_PRINTF(a)
  #endif
  
  struct sem_undo *semu_alloc __P((struct proc *));
  int semundo_adjust __P((struct proc *, struct sem_undo **, int, int, int));
  void semundo_clear __P((int, int));
  
+ /*
+  * XXXSMP Once we go MP, there needs to be a lock for the semaphore system.
+  * Until then, we're saved by being a non-preemptive kernel.
+  */
+ 
  void
  seminit()
  {
***************
*** 55,82 ****
  	semu_list = NULL;
  }
  
- void
- semlock(p)
- 	struct proc *p;
- {
- 
- 	while (semlock_holder != NULL && semlock_holder != p)
- 		sleep((caddr_t)&semlock_holder, (PZERO - 4));
- }
- 
  /*
!  * Lock or unlock the entire semaphore facility.
!  *
!  * This will probably eventually evolve into a general purpose semaphore
!  * facility status enquiry mechanism (I don't like the "read /dev/kmem"
!  * approach currently taken by ipcs and the amount of info that we want
!  * to be able to extract for ipcs is probably beyond the capability of
!  * the getkerninfo facility.
!  *
!  * At the time that the current version of semconfig was written, ipcs is
!  * the only user of the semconfig facility.  It uses it to ensure that the
!  * semaphore facility data structures remain static while it fishes around
!  * in /dev/kmem.
   */
  
  int
--- 58,65 ----
  	semu_list = NULL;
  }
  
  /*
!  * Placebo.
   */
  
  int
***************
*** 85,117 ****
  	void *v;
  	register_t *retval;
  {
- 	struct sys_semconfig_args /* {
- 		syscallarg(int) flag;
- 	} */ *uap = v;
- 	int eval = 0;
- 
- 	semlock(p);
- 
- 	switch (SCARG(uap, flag)) {
- 	case SEM_CONFIG_FREEZE:
- 		semlock_holder = p;
- 		break;
- 
- 	case SEM_CONFIG_THAW:
- 		semlock_holder = NULL;
- 		wakeup((caddr_t)&semlock_holder);
- 		break;
- 
- 	default:
- 		printf(
- 		    "semconfig: unknown flag parameter value (%d) - ignored\n",
- 		    SCARG(uap, flag));
- 		eval = EINVAL;
- 		break;
- 	}
- 
  	*retval = 0;
! 	return(eval);
  }
  
  /*
--- 68,75 ----
  	void *v;
  	register_t *retval;
  {
  	*retval = 0;
! 	return 0;
  }
  
  /*
***************
*** 309,316 ****
  	SEM_PRINTF(("call to semctl(%d, %d, %d, %p)\n",
  	    semid, semnum, cmd, arg));
  
- 	semlock(p);
- 
  	semid = IPCID_TO_IX(semid);
  	if (semid < 0 || semid >= seminfo.semmsl)
  		return(EINVAL);
--- 267,272 ----
***************
*** 467,474 ****
  
  	SEM_PRINTF(("semget(0x%x, %d, 0%o)\n", key, nsems, semflg));
  
- 	semlock(p);
- 
  	if (key != IPC_PRIVATE) {
  		for (semid = 0; semid < seminfo.semmni; semid++) {
  			if ((sema[semid].sem_perm.mode & SEM_ALLOC) &&
--- 423,428 ----
***************
*** 564,571 ****
  
  	SEM_PRINTF(("call to semop(%d, %p, %d)\n", semid, sops, nsops));
  
- 	semlock(p);
- 
  	semid = IPCID_TO_IX(semid);	/* Convert back to zero origin */
  
  	if (semid < 0 || semid >= seminfo.semmsl)
--- 518,523 ----
***************
*** 803,901 ****
  			break;
  	}
  
- 	/*
- 	 * There are a few possibilities to consider here ...
- 	 *
- 	 * 1) The semaphore facility isn't currently locked.  In this case,
- 	 *    this call should proceed normally.
- 	 * 2) The semaphore facility is locked by this process (i.e. the one
- 	 *    that is exiting).  In this case, this call should proceed as
- 	 *    usual and the facility should be unlocked at the end of this
- 	 *    routine (since the locker is exiting).
- 	 * 3) The semaphore facility is locked by some other process and this
- 	 *    process doesn't have an undo structure allocated for it.  In this
- 	 *    case, this call should proceed normally (i.e. not accomplish
- 	 *    anything and, most importantly, not block since that is
- 	 *    unnecessary and could result in a LOT of processes blocking in
- 	 *    here if the facility is locked for a long time).
- 	 * 4) The semaphore facility is locked by some other process and this
- 	 *    process has an undo structure allocated for it.  In this case,
- 	 *    this call should block until the facility has been unlocked since
- 	 *    the holder of the lock may be examining this process's proc entry
- 	 *    (the ipcs utility does this when printing out the information
- 	 *    from the allocated sem undo elements).
- 	 *
- 	 * This leads to the conclusion that we should not block unless we
- 	 * discover that the someone else has the semaphore facility locked and
- 	 * this process has an undo structure.  Let's do that...
- 	 *
- 	 * Note that we do this in a separate pass from the one that processes
- 	 * any existing undo structure since we don't want to risk blocking at
- 	 * that time (it would make the actual unlinking of the element from
- 	 * the chain of allocated undo structures rather messy).
- 	 */
- 
  	/*
! 	 * Does someone else hold the semaphore facility's lock?
  	 */
- 
- 	if (semlock_holder != NULL && semlock_holder != p) {
- 		/*
- 		 * Yes (i.e. we are in case 3 or 4).
- 		 *
- 		 * If we didn't find an undo vector associated with this
- 		 * process than we can just return (i.e. we are in case 3).
- 		 *
- 		 * Note that we know that someone else is holding the lock so
- 		 * we don't even have to see if we're holding it...
- 		 */
- 
- 		if (suptr == NULL)
- 			return;
- 
- 		/*
- 		 * We are in case 4.
- 		 *
- 		 * Go to sleep as long as someone else is locking the semaphore
- 		 * facility (note that we won't get here if we are holding the
- 		 * lock so we don't need to check for that possibility).
- 		 */
- 
- 		while (semlock_holder != NULL)
- 			sleep((caddr_t)&semlock_holder, (PZERO - 4));
- 
- 		/*
- 		 * Nobody is holding the facility (i.e. we are now in case 1).
- 		 * We can proceed safely according to the argument outlined
- 		 * above.
- 		 *
- 		 * We look up the undo vector again, in case the list changed
- 		 * while we were asleep, and the parent is now different.
- 		 */
  
! 		for (supptr = &semu_list; (suptr = *supptr) != NULL;
! 		    supptr = &suptr->un_next) {
! 			if (suptr->un_proc == p)
! 				break;
! 		}
! 
! 		if (suptr == NULL)
! 			panic("semexit: undo vector disappeared");
! 	} else {
! 		/*
! 		 * No (i.e. we are in case 1 or 2).
! 		 *
! 		 * If there is no undo vector, skip to the end and unlock the
! 		 * semaphore facility if necessary.
! 		 */
! 
! 		if (suptr == NULL)
! 			goto unlock;
! 	}
! 
  	/*
! 	 * We are now in case 1 or 2, and we have an undo vector for this
! 	 * process.
  	 */
  
  	SEM_PRINTF(("proc @%p has undo structure with %d entries\n", p,
--- 755,769 ----
  			break;
  	}
  
  	/*
! 	 * If there is no undo vector, skip to the end.
  	 */
  
! 	if (suptr == NULL)
! 		return;
! 	
  	/*
! 	 * We now have an undo vector for this process.
  	 */
  
  	SEM_PRINTF(("proc @%p has undo structure with %d entries\n", p,
***************
*** 946,959 ****
  	SEM_PRINTF(("removing vector\n"));
  	suptr->un_proc = NULL;
  	*supptr = suptr->un_next;
- 
- unlock:
- 	/*
- 	 * If the exiting process is holding the global semaphore facility
- 	 * lock (i.e. we are in case 2) then release it.
- 	 */
- 	if (semlock_holder == p) {
- 		semlock_holder = NULL;
- 		wakeup((caddr_t)&semlock_holder);
- 	}
  }
--- 814,817 ----