Date: Thu, 24 Aug 2000 09:54:30 -0400 (EDT) From: Robert Watson <rwatson@FreeBSD.org> To: Brian Fundakowski Feldman <green@FreeBSD.org> Cc: cvs-committers@FreeBSD.org, cvs-all@FreeBSD.org Subject: Re: cvs commit: src/sys/kern kern_resource.c Message-ID: <Pine.NEB.3.96L.1000824094015.31571C-100000@fledge.watson.org> In-Reply-To: <Pine.BSF.4.21.0008240133530.52828-100000@green.dyndns.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, 24 Aug 2000, Brian Fundakowski Feldman wrote:
> > Err... the first check? No way, man, that's not right. Read the
> > first part of the if (). This check specifically allows root in
> > jail OR out of jail, whereas p_trestpass doesn't. How can you
> > possibly evaluate it to mean otherwise? The first to checks in the if
> > statement...
>
> I hope this didn't come out as sounding flippant and derogatory. I'm
> sorry... tired and feeling sick, don't mind me, please. It still
> stands that I believe my original analysis of that specific part was
> correct, so I'd appreciate more input.
Don't worry about it -- rereading my e-mail, I didn't mean to come out
looking as aggressive as I did, also suffering from somewhat of a lack of
sleep, et al, due to all the travel lately :-).
On the first set of checks, my real concern is that they are not
equivilent:
- if (pcred->pc_ucred->cr_uid && pcred->p_ruid &&
- pcred->pc_ucred->cr_uid != p->p_ucred->cr_uid &&
- pcred->p_ruid != p->p_ucred->cr_uid)
vs.
if (!PRISON_CHECK(p1, p2))
return (ESRCH);
if (p1->p_cred->p_ruid == p2->p_cred->p_ruid)
return (0);
if (p1->p_ucred->cr_uid == p2->p_cred->p_ruid)
return (0);
if (p1->p_cred->p_ruid == p2->p_ucred->cr_uid)
return (0);
if (p1->p_ucred->cr_uid == p2->p_ucred->cr_uid)
return (0);
The new check allows processes to do rtprio stuff in situations where
previously, they could not. Part of this is because the whole multiple
uid cred thing is getting a bit out of hand (due to POSIX, et al). I
misinterpretted in saying that the out-of-jail check has been removed, as
the later suser_xxx() call happens after a PRISON_CHECK(), and the old
code did not have the PRISON_CHECK() call (just my patches to it, hence my
thinking it had now become broken :-).
Now, I'm actually not claiming the permissions change here isn't good, as
making our checks more consistent is "good". However, we probably need to
figure out if these changes are a problem or not--(a) whether we violate
any specs through these changes, and (b) whether the permission changes
now allow a class of processes to change their real time priority that
should not be able to.
Frankly, all these "your uid equals their uid" checks in different forms
irritate me, and I'd rather see a migration towards consistent checks.
What I should do is bundle up my patches for this code (essentially, a set
of calls, p_can_kill(), p_can_see(), p_can_debug(), p_can_schedule(), etc)
and we can merge them into the source base to simplify things. Many of
the p_can_* calls actually invoke p_can_see(), which effectively breaks
out the prison check from the super-user check. In my mind, the prison
check is really a scoping issue rather than strictly a invoking privilege
check.
Robert N M Watson
robert@fledge.watson.org http://www.watson.org/~robert/
PGP key fingerprint: AF B5 5F FF A6 4A 79 37 ED 5F 55 E9 58 04 6A B1
TIS Labs at Network Associates, Safeport Network Services
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe cvs-all" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?Pine.NEB.3.96L.1000824094015.31571C-100000>