Date: Fri, 9 Jun 2000 16:44:08 -0700 From: Alfred Perlstein <bright@wintelcom.net> To: hackers@freebsd.org Cc: green@freebsd.org Subject: uidinfo has many race conditions. Message-ID: <20000609164408.N18462@fw.wintelcom.net>
next in thread | raw e-mail | index | archive | help
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 majordomo@FreeBSD.org
with "unsubscribe freebsd-hackers" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20000609164408.N18462>