From owner-cvs-all  Wed Jan 24  9:55:30 2001
Delivered-To: cvs-all@freebsd.org
Received: from fw.wintelcom.net (ns1.wintelcom.net [209.1.153.20])
	by hub.freebsd.org (Postfix) with ESMTP
	id B843737B404; Wed, 24 Jan 2001 09:55:04 -0800 (PST)
Received: (from bright@localhost)
	by fw.wintelcom.net (8.10.0/8.10.0) id f0OHt4v13372;
	Wed, 24 Jan 2001 09:55:04 -0800 (PST)
Date: Wed, 24 Jan 2001 09:55:04 -0800
From: Alfred Perlstein <bright@wintelcom.net>
To: John Baldwin <jhb@FreeBSD.org>
Cc: cvs-committers@FreeBSD.org, cvs-all@FreeBSD.org
Subject: Re: cvs commit: src/sys/alpha/osf1 osf1_misc.c osf1_signal.c
Message-ID: <20010124095504.X26076@fw.wintelcom.net>
References: <200101241027.f0OARBH07561@freefall.freebsd.org>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
User-Agent: Mutt/1.2.5i
In-Reply-To: <200101241027.f0OARBH07561@freefall.freebsd.org>; from jhb@FreeBSD.org on Wed, Jan 24, 2001 at 02:27:11AM -0800
Sender: owner-cvs-all@FreeBSD.ORG
Precedence: bulk
X-Loop: FreeBSD.ORG

* John Baldwin <jhb@FreeBSD.org> [010124 02:28] wrote:
> jhb         2001/01/24 02:27:11 PST
> 
>   Modified files:
>     sys/alpha/osf1       osf1_misc.c osf1_signal.c 
>   Log:
>   Proc locking.

I thought you said you would stop locking read only structures?

When I say "read-only" I mean from the context of other threads,
it's ok if a process changes its own p_cred, and no one should
be changing it out from under it.

If someone else is fiddling with another proc'd p_cred it
just needs to make sure that the process can't exit before
it's done.

        uid = SCARG(uap, uid);
+       PROC_LOCK(p);
        pc = p->p_cred;
 
        if ((error = suser(p)) != 0 &&
-           uid != pc->p_ruid && uid != pc->p_svuid)
+           uid != pc->p_ruid && uid != pc->p_svuid) {
+               PROC_UNLOCK(p);
                return (error);
+       }
 
        if (error == 0) {
                if (uid != pc->p_ruid) {
+                       PROC_UNLOCK(p);
                        change_ruid(p, uid);
                        setsugid(p);
+                       PROC_LOCK(p);
                }
                if (pc->p_svuid != uid) {
+                       PROC_UNLOCK(p);
                        pc->p_svuid = uid;
                        setsugid(p);
+                       PROC_LOCK(p);
                }
        }
        if (pc->pc_ucred->cr_uid != uid) {
+               PROC_UNLOCK(p);
                change_euid(p, uid);
                setsugid(p);
-       }
+       } else
+               PROC_UNLOCK(p);
        return (0);
 }

pc = p->p_cred;

p->p_cred is read only, it is not shared between processes and
should never have a refcount > 1, the refcount is actually bogus
except in BSD/os where it is incorrectly assumed to be so.

We (currently) don't have this problem.

-- 
-Alfred Perlstein - [bright@wintelcom.net|alfred@freebsd.org]
"I have the heart of a child; I keep it in a jar on my desk."


To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe cvs-all" in the body of the message