summaryrefslogtreecommitdiffstats
path: root/sys/kern
diff options
context:
space:
mode:
authoralfred <alfred@FreeBSD.org>2000-06-22 22:27:16 +0000
committeralfred <alfred@FreeBSD.org>2000-06-22 22:27:16 +0000
commit7f71a1a09199f766bb8569761d4cb15d13abdc22 (patch)
tree331e9f18a5ec115951f1cd9af64ffd33f945ed61 /sys/kern
parent8b986caf33fc972c768af8986cc1cec3e794a9d4 (diff)
downloadFreeBSD-src-7f71a1a09199f766bb8569761d4cb15d13abdc22.zip
FreeBSD-src-7f71a1a09199f766bb8569761d4cb15d13abdc22.tar.gz
fix races in the uidinfo subsystem, several problems existed:
1) while allocating a uidinfo struct malloc is called with M_WAITOK, it's possible that while asleep another process by the same user could have woken up earlier and inserted an entry into the uid hash table. Having redundant entries causes inconsistancies that we can't handle. fix: do a non-waiting malloc, and if that fails then do a blocking malloc, after waking up check that no one else has inserted an entry for us already. 2) Because many checks for sbsize were done as "test then set" in a non atomic manner it was possible to exceed the limits put up via races. fix: instead of querying the count then setting, we just attempt to set the count and leave it up to the function to return success or failure. 3) The uidinfo code was inlining and repeating, lookups and insertions and deletions needed to be in their own functions for clarity. Reviewed by: green
Diffstat (limited to 'sys/kern')
-rw-r--r--sys/kern/init_main.c2
-rw-r--r--sys/kern/kern_exit.c2
-rw-r--r--sys/kern/kern_fork.c7
-rw-r--r--sys/kern/kern_proc.c146
-rw-r--r--sys/kern/kern_prot.c12
-rw-r--r--sys/kern/uipc_sockbuf.c8
-rw-r--r--sys/kern/uipc_socket.c4
-rw-r--r--sys/kern/uipc_socket2.c8
-rw-r--r--sys/kern/uipc_usrreq.c5
9 files changed, 116 insertions, 78 deletions
diff --git a/sys/kern/init_main.c b/sys/kern/init_main.c
index b732b90..442d266 100644
--- a/sys/kern/init_main.c
+++ b/sys/kern/init_main.c
@@ -387,7 +387,7 @@ proc0_init(dummy)
/*
* Charge root for one process.
*/
- (void)chgproccnt(0, 1);
+ (void)chgproccnt(0, 1, 0);
/*
* Initialize the current process pointer (curproc) before
diff --git a/sys/kern/kern_exit.c b/sys/kern/kern_exit.c
index 9115f97..22e25b1 100644
--- a/sys/kern/kern_exit.c
+++ b/sys/kern/kern_exit.c
@@ -472,7 +472,7 @@ loop:
/*
* Decrement the count of procs running with this uid.
*/
- (void)chgproccnt(p->p_cred->p_ruid, -1);
+ (void)chgproccnt(p->p_cred->p_ruid, -1, 0);
/*
* Release reference to text vnode
diff --git a/sys/kern/kern_fork.c b/sys/kern/kern_fork.c
index 62296b2..58e7ecc 100644
--- a/sys/kern/kern_fork.c
+++ b/sys/kern/kern_fork.c
@@ -183,7 +183,7 @@ fork1(p1, flags, procp)
struct proc *p2, *pptr;
uid_t uid;
struct proc *newproc;
- int count;
+ int ok;
static int pidchecked = 0;
struct forklist *ep;
@@ -245,9 +245,8 @@ fork1(p1, flags, procp)
* Increment the count of procs running with this uid. Don't allow
* a nonprivileged user to exceed their current limit.
*/
- count = chgproccnt(uid, 1);
- if (uid != 0 && count > p1->p_rlimit[RLIMIT_NPROC].rlim_cur) {
- (void)chgproccnt(uid, -1);
+ ok = chgproccnt(uid, 1, p1->p_rlimit[RLIMIT_NPROC].rlim_cur);
+ if (uid != 0 && !ok) {
/*
* Back out the process count
*/
diff --git a/sys/kern/kern_proc.c b/sys/kern/kern_proc.c
index df9f1c5..72069d0 100644
--- a/sys/kern/kern_proc.c
+++ b/sys/kern/kern_proc.c
@@ -72,6 +72,10 @@ static u_long uihash; /* size of hash table - 1 */
static void orphanpg __P((struct pgrp *pg));
+static struct uidinfo *uifind(uid_t uid);
+static struct uidinfo *uicreate(uid_t uid);
+static int uifree(struct uidinfo *uip);
+
/*
* Other process lists
*/
@@ -99,85 +103,119 @@ procinit()
}
/*
- * Change the count associated with number of processes
- * a given user is using.
+ * find/create a uidinfo struct for the uid passed in
*/
-int
-chgproccnt(uid, diff)
- uid_t uid;
- int diff;
+static struct uidinfo *
+uifind(uid)
+ uid_t uid;
{
- register struct uidinfo *uip;
- register struct uihashhead *uipp;
+ struct uihashhead *uipp;
+ struct uidinfo *uip;
uipp = UIHASH(uid);
LIST_FOREACH(uip, uipp, ui_hash)
if (uip->ui_uid == uid)
break;
- 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);
+
+ return (uip);
+}
+
+static struct uidinfo *
+uicreate(uid)
+ uid_t uid;
+{
+ struct uidinfo *uip, *norace;
+
+ MALLOC(uip, struct uidinfo *, sizeof(*uip), M_PROC, M_NOWAIT);
+ if (uip == NULL) {
+ 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);
+ }
+ }
+ LIST_INSERT_HEAD(UIHASH(uid), uip, ui_hash);
+ uip->ui_uid = uid;
+ uip->ui_proccnt = 0;
+ uip->ui_sbsize = 0;
+ return (uip);
+}
+
+static int
+uifree(uip)
+ struct uidinfo *uip;
+{
+
+ if (uip->ui_sbsize == 0 && uip->ui_proccnt == 0) {
LIST_REMOVE(uip, ui_hash);
FREE(uip, M_PROC);
- return (0);
+ return (1);
}
- if (diff <= 0) {
- if (diff == 0)
- return(0);
- panic("chgproccnt: lost user");
+ return (0);
+}
+
+/*
+ * Change the count associated with number of processes
+ * a given user is using. When 'max' is 0, don't enforce a limit
+ */
+int
+chgproccnt(uid, diff, max)
+ uid_t uid;
+ int diff;
+ int max;
+{
+ struct uidinfo *uip;
+
+ uip = uifind(uid);
+ if (diff < 0)
+ KASSERT(uip != NULL, ("reducing proccnt: lost count, uid = %d", uid));
+ if (uip == NULL)
+ uip = uicreate(uid);
+ /* don't allow them to exceed max, but allow subtraction */
+ if (diff > 0 && uip->ui_proccnt + diff > max && max != 0) {
+ (void)uifree(uip);
+ return (0);
}
- MALLOC(uip, struct uidinfo *, sizeof(*uip), M_PROC, M_WAITOK);
- LIST_INSERT_HEAD(uipp, uip, ui_hash);
- uip->ui_uid = uid;
- uip->ui_proccnt = diff;
- uip->ui_sbsize = 0;
- return (diff);
+ uip->ui_proccnt += diff;
+ (void)uifree(uip);
+ return (1);
}
/*
* Change the total socket buffer size a user has used.
*/
-rlim_t
-chgsbsize(uid, diff)
+int
+chgsbsize(uid, diff, max)
uid_t uid;
rlim_t diff;
+ rlim_t max;
{
- register struct uidinfo *uip;
- register struct uihashhead *uipp;
-
- uipp = UIHASH(uid);
- LIST_FOREACH(uip, uipp, ui_hash)
- if (uip->ui_uid == uid)
- break;
- if (diff <= 0) {
- if (diff == 0)
- return (uip ? uip->ui_sbsize : 0);
- KASSERT(uip != NULL, ("uidinfo (%d) gone", uid));
- }
- 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);
+ struct uidinfo *uip;
+
+ uip = uifind(uid);
+ if (diff < 0)
+ KASSERT(uip != NULL, ("reducing sbsize: lost count, uid = %d", uid));
+ if (uip == NULL)
+ uip = uicreate(uid);
+ /* don't allow them to exceed max, but allow subtraction */
+ if (diff > 0 && uip->ui_sbsize + diff > max) {
+ (void)uifree(uip);
+ return (0);
}
- 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->ui_sbsize = diff;
- return (diff);
+ uip->ui_sbsize += diff;
+ (void)uifree(uip);
+ return (1);
}
/*
* Is p an inferior of the current process?
*/
-int
+inT
inferior(p)
register struct proc *p;
{
diff --git a/sys/kern/kern_prot.c b/sys/kern/kern_prot.c
index e001e2e..0a8ae2c 100644
--- a/sys/kern/kern_prot.c
+++ b/sys/kern/kern_prot.c
@@ -430,8 +430,8 @@ setuid(p, uap)
* Transfer proc count to new user.
*/
if (uid != pc->p_ruid) {
- (void)chgproccnt(pc->p_ruid, -1);
- (void)chgproccnt(uid, 1);
+ (void)chgproccnt(pc->p_ruid, -1, 0);
+ (void)chgproccnt(uid, 1, 0);
}
/*
* Set real uid
@@ -679,8 +679,8 @@ setreuid(p, uap)
setsugid(p);
}
if (ruid != (uid_t)-1 && pc->p_ruid != ruid) {
- (void)chgproccnt(pc->p_ruid, -1);
- (void)chgproccnt(ruid, 1);
+ (void)chgproccnt(pc->p_ruid, -1, 0);
+ (void)chgproccnt(ruid, 1, 0);
pc->p_ruid = ruid;
setsugid(p);
}
@@ -772,8 +772,8 @@ setresuid(p, uap)
setsugid(p);
}
if (ruid != (uid_t)-1 && pc->p_ruid != ruid) {
- (void)chgproccnt(pc->p_ruid, -1);
- (void)chgproccnt(ruid, 1);
+ (void)chgproccnt(pc->p_ruid, -1, 0);
+ (void)chgproccnt(ruid, 1, 0);
pc->p_ruid = ruid;
setsugid(p);
}
diff --git a/sys/kern/uipc_sockbuf.c b/sys/kern/uipc_sockbuf.c
index 42e02a5..beb8fe5 100644
--- a/sys/kern/uipc_sockbuf.c
+++ b/sys/kern/uipc_sockbuf.c
@@ -432,10 +432,10 @@ sbreserve(sb, cc, so, p)
if ((u_quad_t)cc > (u_quad_t)sb_max * MCLBYTES / (MSIZE + MCLBYTES))
return (0);
delta = (rlim_t)cc - sb->sb_hiwat;
- if (p && delta >= 0 && chgsbsize(so->so_cred->cr_uid, 0) + delta >
- p->p_rlimit[RLIMIT_SBSIZE].rlim_cur)
+ if (p && !chgsbsize(so->so_cred->cr_uid, delta,
+ p->p_rlimit[RLIMIT_SBSIZE].rlim_cur)) {
return (0);
- (void)chgsbsize(so->so_cred->cr_uid, delta);
+ }
sb->sb_hiwat = cc;
sb->sb_mbmax = min(cc * sb_efficiency, sb_max);
if (sb->sb_lowat > sb->sb_hiwat)
@@ -453,7 +453,7 @@ sbrelease(sb, so)
{
sbflush(sb);
- (void)chgsbsize(so->so_cred->cr_uid, -(rlim_t)sb->sb_hiwat);
+ (void)chgsbsize(so->so_cred->cr_uid, -(rlim_t)sb->sb_hiwat, RLIM_INFINITY);
sb->sb_hiwat = sb->sb_mbmax = 0;
}
diff --git a/sys/kern/uipc_socket.c b/sys/kern/uipc_socket.c
index c2d0ad7..4dffd3a 100644
--- a/sys/kern/uipc_socket.c
+++ b/sys/kern/uipc_socket.c
@@ -191,10 +191,10 @@ sodealloc(so)
so->so_gencnt = ++so_gencnt;
if (so->so_rcv.sb_hiwat)
(void)chgsbsize(so->so_cred->cr_uid,
- -(rlim_t)so->so_rcv.sb_hiwat);
+ -(rlim_t)so->so_rcv.sb_hiwat, RLIM_INFINITY);
if (so->so_snd.sb_hiwat)
(void)chgsbsize(so->so_cred->cr_uid,
- -(rlim_t)so->so_snd.sb_hiwat);
+ -(rlim_t)so->so_snd.sb_hiwat, RLIM_INFINITY);
if (so->so_accf != NULL) {
if (so->so_accf->so_accept_filter != NULL &&
so->so_accf->so_accept_filter->accf_destroy != NULL) {
diff --git a/sys/kern/uipc_socket2.c b/sys/kern/uipc_socket2.c
index 42e02a5..beb8fe5 100644
--- a/sys/kern/uipc_socket2.c
+++ b/sys/kern/uipc_socket2.c
@@ -432,10 +432,10 @@ sbreserve(sb, cc, so, p)
if ((u_quad_t)cc > (u_quad_t)sb_max * MCLBYTES / (MSIZE + MCLBYTES))
return (0);
delta = (rlim_t)cc - sb->sb_hiwat;
- if (p && delta >= 0 && chgsbsize(so->so_cred->cr_uid, 0) + delta >
- p->p_rlimit[RLIMIT_SBSIZE].rlim_cur)
+ if (p && !chgsbsize(so->so_cred->cr_uid, delta,
+ p->p_rlimit[RLIMIT_SBSIZE].rlim_cur)) {
return (0);
- (void)chgsbsize(so->so_cred->cr_uid, delta);
+ }
sb->sb_hiwat = cc;
sb->sb_mbmax = min(cc * sb_efficiency, sb_max);
if (sb->sb_lowat > sb->sb_hiwat)
@@ -453,7 +453,7 @@ sbrelease(sb, so)
{
sbflush(sb);
- (void)chgsbsize(so->so_cred->cr_uid, -(rlim_t)sb->sb_hiwat);
+ (void)chgsbsize(so->so_cred->cr_uid, -(rlim_t)sb->sb_hiwat, RLIM_INFINITY);
sb->sb_hiwat = sb->sb_mbmax = 0;
}
diff --git a/sys/kern/uipc_usrreq.c b/sys/kern/uipc_usrreq.c
index 5c0e8238..38ac3b0 100644
--- a/sys/kern/uipc_usrreq.c
+++ b/sys/kern/uipc_usrreq.c
@@ -48,6 +48,7 @@
#include <sys/protosw.h>
#include <sys/socket.h>
#include <sys/socketvar.h>
+#include <sys/resourcevar.h>
#include <sys/stat.h>
#include <sys/sysctl.h>
#include <sys/un.h>
@@ -236,7 +237,7 @@ uipc_rcvd(struct socket *so, int flags)
unp->unp_mbcnt = so->so_rcv.sb_mbcnt;
so2->so_snd.sb_hiwat += unp->unp_cc - so->so_rcv.sb_cc;
(void)chgsbsize(so2->so_cred->cr_uid,
- (rlim_t)unp->unp_cc - so->so_rcv.sb_cc);
+ (rlim_t)unp->unp_cc - so->so_rcv.sb_cc, RLIM_INFINITY);
unp->unp_cc = so->so_rcv.sb_cc;
sowwakeup(so2);
break;
@@ -344,7 +345,7 @@ uipc_send(struct socket *so, int flags, struct mbuf *m, struct sockaddr *nam,
so->so_snd.sb_hiwat -=
so2->so_rcv.sb_cc - unp->unp_conn->unp_cc;
(void)chgsbsize(so->so_cred->cr_uid,
- (rlim_t)unp->unp_conn->unp_cc - so2->so_rcv.sb_cc);
+ (rlim_t)unp->unp_conn->unp_cc - so2->so_rcv.sb_cc, RLIM_INFINITY);
unp->unp_conn->unp_cc = so2->so_rcv.sb_cc;
sorwakeup(so2);
m = 0;
OpenPOWER on IntegriCloud