hi,

Is it just me or does the fact that uidinfo structures (see
kern/kern_proc.c) are allocated with M_WAITOK after finding them
fails and then inserted into the uidhash structure a race condition?

There's also a problem with sbsize checking because of races going on
here, what needs to happen is that the changeXXsize/count functions
need to know what they are chenging and doing them without races.

I should have some diffs up soon that address this.

here's fixing the uidinfo stuff, but it still doesn't fix that
the sbsize checks before setting the size, which needs to be
done atomically.  I think the solution is to have chgsbsize
actually check the rlimit instead of polling then setting.

chblob is for my own stuff.


Index: kern_proc.c
===================================================================
RCS file: /home/ncvs/src/sys/kern/kern_proc.c,v
retrieving revision 1.63
diff -u -u -r1.63 kern_proc.c
--- kern_proc.c 2000/02/08 19:54:15     1.63
+++ kern_proc.c 2000/06/09 23:40:46
@@ -56,6 +56,9 @@
 MALLOC_DEFINE(M_SUBPROC, "subproc", "Proc sub-structures");
 
 static void pgdelete   __P((struct pgrp *));
+static struct uidinfo *uifind(uid_t uid);
+static struct uidinfo *uicreate(uid_t uid);
+static int uifree(struct uidinfo *uip);
 
 /*
  * Structure associated with user cacheing.
@@ -65,6 +68,7 @@
        uid_t   ui_uid;
        long    ui_proccnt;
        rlim_t  ui_sbsize;
+       rlim_t  ui_blobsize;
 };
 #define        UIHASH(uid)     (&uihashtbl[(uid) & uihash])
 static LIST_HEAD(uihashhead, uidinfo) *uihashtbl;
@@ -99,6 +103,57 @@
 }
 
 /*
+ * find/create a uidinfo struct for the uid passed in
+ */
+static struct uidinfo *
+uifind(uid_t uid)
+{
+       struct uihashhead *uipp;
+       struct uidinfo *uip;
+
+       uipp = UIHASH(uid);
+       LIST_FOREACH(uip, uipp, ui_hash)
+               if (uip->ui_uid == uid)
+                       break;
+
+       return (uip);
+}
+
+static struct uidinfo *
+uicreate(uid_t uid)
+{
+       struct uidinfo *uip, *norace;
+
+       MALLOC(uip, struct uidinfo *, sizeof(*uip), M_PROC, M_WAITOK);
+       /*
+        * if we M_WAITOK we must look afterwards or risk redundant entries
+        */
+       norace = uifind(uid);
+       if (norace != NULL) {
+               FREE(uip, M_PROC);
+               return (norace);
+       }
+       uip->ui_uid = uid;
+       uip->ui_proccnt = 0;
+       uip->ui_sbsize = 0;
+       uip->ui_blobsize = 0;
+       return (uip);
+}
+
+static int
+uifree(struct uidinfo *uip)
+{
+
+       if (uip->ui_sbsize == 0 && uip->ui_proccnt == 0 && uip->ui_blobsize == 0) {
+               LIST_REMOVE(uip, ui_hash);
+               FREE(uip, M_PROC);
+               return (1);
+       }
+       return (0);
+}
+
+
+/*
  * Change the count associated with number of processes
  * a given user is using.
  */
@@ -108,35 +163,25 @@
        int     diff;
 {
        register struct uidinfo *uip;
-       register struct uihashhead *uipp;
 
-       uipp = UIHASH(uid);
-       LIST_FOREACH(uip, uipp, ui_hash)
-               if (uip->ui_uid == uid)
-                       break;
+       uip = uifind(uid);
        if (uip) {
                uip->ui_proccnt += diff;
                if (uip->ui_proccnt < 0)
                        panic("chgproccnt: procs < 0");
-               if (uip->ui_proccnt > 0 || uip->ui_sbsize > 0)
-                       return (uip->ui_proccnt);
-               LIST_REMOVE(uip, ui_hash);
-               FREE(uip, M_PROC);
-               return (0);
+               return (uifree(uip) == 1 ? 0 : uip->ui_proccnt);
        }
        if (diff <= 0) {
                if (diff == 0)
                        return(0);
                panic("chgproccnt: lost user");
        }
-       MALLOC(uip, struct uidinfo *, sizeof(*uip), M_PROC, M_WAITOK);
-       LIST_INSERT_HEAD(uipp, uip, ui_hash);
-       uip->ui_uid = uid;
+       uip = uicreate(uid);
        uip->ui_proccnt = diff;
-       uip->ui_sbsize = 0;
        return (diff);
 }
 
+
 /*
  * Change the total socket buffer size a user has used.
  */
@@ -146,12 +191,8 @@
        rlim_t  diff;
 {
        register struct uidinfo *uip;
-       register struct uihashhead *uipp;
 
-       uipp = UIHASH(uid);
-       LIST_FOREACH(uip, uipp, ui_hash)
-               if (uip->ui_uid == uid)
-                       break;
+       uip = uifind(uid);
        if (diff <= 0) {
                if (diff == 0)
                        return (uip ? uip->ui_sbsize : 0);
@@ -159,20 +200,38 @@
        }
        if (uip) {
                uip->ui_sbsize += diff;
-               if (uip->ui_sbsize == 0 && uip->ui_proccnt == 0) {
-                       LIST_REMOVE(uip, ui_hash);
-                       FREE(uip, M_PROC);
-                       return (0);
-               }
-               return (uip->ui_sbsize);
+               return (uifree(uip) == 1 ? 0 : uip->ui_sbsize);
        }
-       MALLOC(uip, struct uidinfo *, sizeof(*uip), M_PROC, M_WAITOK);
-       LIST_INSERT_HEAD(uipp, uip, ui_hash);
-       uip->ui_uid = uid;
-       uip->ui_proccnt = 0;
+       uip = uicreate(uid);
        uip->ui_sbsize = diff;
        return (diff);
 }
+
+/*
+ * Change the total amount of kblob space a suser can consume
+ */
+rlim_t
+chgblobsize(uid, diff)
+       uid_t   uid;
+       rlim_t  diff;
+{
+       register struct uidinfo *uip;
+
+       uip = uifind(uid);
+       if (diff <= 0) {
+               if (diff == 0)
+                       return (uip ? uip->ui_blobsize : 0);
+               KASSERT(uip != NULL, ("uidinfo (%d) gone", uid));
+       }
+       if (uip) {
+               uip->ui_blobsize += diff;
+               return (uifree(uip) == 1 ? 0 : uip->ui_blobsize);
+       }
+       uicreate(uid);
+       uip->ui_blobsize = diff;
+       return (diff);
+}
+
 
 /*
  * Is p an inferior of the current process?



To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-hackers" in the body of the message

Reply via email to