Re: semctl(2) SETVAL/SETALL does not validate the semaphore value

[Nicolas Joly <njoly%pasteur.fr@localhost>]

On Mon, May 05, 2008 at 11:34:17AM +0200, Matthias Drochner wrote:
> 
> njoly%pasteur.fr@localhost said:
> >             for (i = 0; i < semaptr->sem_nsems; i++) {
> > +                   if ((unsigned int)arg->array[i] > seminfo.semvmx) { 
> 
> copyin before test?

Nice catch, thanks ... I didn't test that part, but blindly adapted it
from SETVAL case.

Here follow an updated version.

-- 
Nicolas Joly

Biological Software and Databanks.
Institut Pasteur, Paris.

Index: sys/kern/sysv_sem.c
===================================================================
RCS file: /cvsroot/src/sys/kern/sysv_sem.c,v
retrieving revision 1.82
diff -u -p -r1.82 sysv_sem.c
--- sys/kern/sysv_sem.c 28 Apr 2008 20:24:05 -0000      1.82
+++ sys/kern/sysv_sem.c 5 May 2008 10:13:40 -0000
@@ -603,6 +603,10 @@ semctl1(struct lwp *l, int semid, int se
                        break;
                }
                KASSERT(arg != NULL);
+               if ((unsigned int)arg->val > seminfo.semvmx) {
+                       error = ERANGE;
+                       break;
+               }
                semaptr->_sem_base[semnum].semval = arg->val;
                semundo_clear(ix, semnum);
                cv_broadcast(&semcv[ix]);
@@ -613,11 +617,16 @@ semctl1(struct lwp *l, int semid, int se
                        break;
                KASSERT(arg != NULL);
                for (i = 0; i < semaptr->sem_nsems; i++) {
-                       error = copyin(&arg->array[i],
-                           &semaptr->_sem_base[i].semval,
+                       unsigned short semval;
+                       error = copyin(&arg->array[i], &semval,
                            sizeof(arg->array[i]));
                        if (error != 0)
                                break;
+                       if ((unsigned int)semval > seminfo.semvmx) {
+                               error = ERANGE;
+                               break;
+                       semaptr->_sem_base[i].semval = semval;
+                       }
                }
                semundo_clear(ix, -1);
                cv_broadcast(&semcv[ix]);
Index: lib/libc/sys/semctl.2
===================================================================
RCS file: /cvsroot/src/lib/libc/sys/semctl.2,v
retrieving revision 1.16
diff -u -p -r1.16 semctl.2
--- lib/libc/sys/semctl.2       13 May 2004 10:20:58 -0000      1.16
+++ lib/libc/sys/semctl.2       5 May 2008 10:13:40 -0000
@@ -223,6 +223,13 @@ is not a valid command.
 or
 .Fa arg.array
 specifies an invalid address.
+.It Bq Er ERANGE
+.Fa cmd
+is equal to
+.Dv SETVAL
+or
+.Dv SETALL
+and the value to be set is greater than the system semaphore maximum value.
 .El
 .Sh SEE ALSO
 .Xr semget 2 ,