Date: Thu, 2 Mar 2000 09:58:13 -0500 (EST) From: Brian Dean <brdean@unx.sas.com> To: Bruce Evans <bde@zeta.org.au> Cc: Kris Kennaway <kris@hub.freebsd.org>, current@FreeBSD.ORG Subject: Re: HEADS UP! IPC security (Re: cvs commit: src/sys/kern sysv_ipc.c (fwd)) Message-ID: <200003021458.JAA00436@dean.pc.sas.com> In-Reply-To: <Pine.BSF.4.21.0003021742060.1078-100000@alphplex.bde.org> from Bruce Evans at "Mar 2, 2000 06:26:22 pm"
next in thread | previous in thread | raw e-mail | index | archive | help
Bruce Evans wrote:
> It had rotted further since then (as half threatened in rev.1.9) to
> passing the process pointer so that it can mess up p->p_acflag. This
> results in the ASU flag always being set in p->p_acflag for root,
> although no special privilege is required for root to operate on ipc
> objects owned by root, and especially, no special privilege is required
> to determine whether an operation is permitted. The ASU flag should
> only be set if root privilege is used. Most callers of ipcperm() are
> committed to doing the operation that they check for using ipcperm()
> if ipcperm() succeeds, so rearrangement of ipcperm() to check for root
> privilege last would fix most cases.
[...]
> Now the code has rotted to having an almost unused variable and one
> other style bug:
>
> int error;
> ...
> error = suser(p);
> if (!error)
>
> This should be written much like the check in rev.1.8:
>
> if (suser(p) == 0)
I believe the following patch does what you are asking. Essentially,
it only calls suser() if it was about to return a permission error,
thus the ASU flag should only be set when superuser privileges are
actually used.
Let me know if this looks OK, and if Jordan approves, I'll commit it.
Thanks,
-Brian
--
Brian Dean brdean@unx.sas.com
SAS Institute Inc. bsd@FreeBSD.ORG
Index: sysv_ipc.c
===================================================================
RCS file: /usr00/mirror/ncvs/src/sys/kern/sysv_ipc.c,v
retrieving revision 1.13
diff -u -r1.13 sysv_ipc.c
--- sysv_ipc.c 2000/02/29 22:58:59 1.13
+++ sysv_ipc.c 2000/03/02 14:39:41
@@ -51,16 +51,15 @@
int mode;
{
struct ucred *cred = p->p_ucred;
- int error;
- error = suser(p);
- if (!error)
- return (0);
-
/* Check for user match. */
if (cred->cr_uid != perm->cuid && cred->cr_uid != perm->uid) {
- if (mode & IPC_M)
- return (EPERM);
+ if (mode & IPC_M) {
+ if (suser(p) == 0)
+ return (0);
+ else
+ return (EPERM);
+ }
/* Check for group match. */
mode >>= 3;
if (!groupmember(perm->gid, cred) &&
@@ -71,7 +70,14 @@
if (mode & IPC_M)
return (0);
- return ((mode & perm->mode) == mode ? 0 : EACCES);
+
+ if ((mode & perm->mode) == mode)
+ return (0);
+
+ if (suser(p) == 0)
+ return (0);
+
+ return EACCES;
}
#endif /* defined(SYSVSEM) || defined(SYSVSHM) || defined(SYSVMSG) */
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-current" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200003021458.JAA00436>