From owner-freebsd-fs Sun Aug 27 8:30:58 2000 Delivered-To: freebsd-fs@freebsd.org Received: from fledge.watson.org (fledge.watson.org [204.156.12.50]) by hub.freebsd.org (Postfix) with ESMTP id 03DA437B42C; Sun, 27 Aug 2000 08:30:49 -0700 (PDT) Received: from fledge.watson.org (robert@fledge.pr.watson.org [192.0.2.3]) by fledge.watson.org (8.9.3/8.9.3) with SMTP id LAA71933; Sun, 27 Aug 2000 11:30:48 -0400 (EDT) (envelope-from robert@fledge.watson.org) Date: Sun, 27 Aug 2000 11:30:47 -0400 (EDT) From: Robert Watson X-Sender: robert@fledge.watson.org To: phk@FreeBSD.org Cc: freebsd-fs@FreeBSD.org Subject: procfs_lookup() and jail interaction Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-fs@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.org Poul-Henning, While propagating changes to process access control, I noticed that I was getting a warning in procfs_vnops.c, in procfs_lookup(). It turned out, I replaced a PRISON_CHECK() with a p_cansee(), and as a macro, PRISON_CHECK didn't have any problem with processing on a "const" struct proc, whereas my p_cansee() does, as it calls suser(). Apparently, the call is made directly on curproc, rather than on the struct proc passed in via cnp, as part of the name lookup. So my first question is: why don't the access control checks in procfs_lookup() relating to jail make use of the struct proc passed in, rather than curproc. My second question has to do with namespace caching. My initial interpretation of this code was that the access control check only happens for the first lookup, and after it's successfully in the name cache, any process can tell that the name exists, even if they can't access it. This occurs because (a) caching access control decisions in the name cache probably doesn't make sense, and (b) our name cache and file system arrangement was not intended to support hidden names in a readable directory. That's not to say we can't hide it in readdir(), just that from the lookup perspective, our vfs just doesn't work that way. Making it work that way would probably be a mistake, as we'd lose many of the performance advantages of our name cache. That said, I'd like to consider what changes should be made in procfs_lookup(). If my glance and understanding are correct: that is, that the lookup protection there is somewhat faulty anyway due to the name cache, should I just eliminate the check? Or alternatively, to clear up the warning, should I make use of a_cnp->curp, which will not be a const struct proc *? Thanks, 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 freebsd-fs" in the body of the message From owner-freebsd-fs Mon Aug 28 9:40:20 2000 Delivered-To: freebsd-fs@freebsd.org Received: from fledge.watson.org (fledge.watson.org [204.156.12.50]) by hub.freebsd.org (Postfix) with ESMTP id 8F46537B424 for ; Mon, 28 Aug 2000 09:40:17 -0700 (PDT) Received: from fledge.watson.org (robert@fledge.pr.watson.org [192.0.2.3]) by fledge.watson.org (8.9.3/8.9.3) with SMTP id MAA85726 for ; Mon, 28 Aug 2000 12:40:16 -0400 (EDT) (envelope-from robert@fledge.watson.org) Date: Mon, 28 Aug 2000 12:40:16 -0400 (EDT) From: Robert Watson X-Sender: robert@fledge.watson.org To: freebsd-fs@FreeBSD.org Subject: volatile struct proc pointer in procfs_vnops.o Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-fs@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.org I'm getting a couple of warnings compiling procfs_vnops.o with p_cansee() instead of PRISON_CHECK(), as the struct proc *p is qualified as volatile: int pcnt = 0; volatile struct proc *p = allproc.lh_first; ... while (pcnt < i) { p = p->p_list.le_next; if (!p) goto done; if (p_cansee(curproc, p, NULL)) continue; pcnt++; } ../../miscfs/procfs/procfs_vnops.c: In function `procfs_readdir': ../../miscfs/procfs/procfs_vnops.c:882: warning: passing arg 2 of `p_cansee' discards qualifiers from pointer target type ../../miscfs/procfs/procfs_vnops.c:886: warning: passing arg 2 of `p_cansee' discards qualifiers from pointer target type Does anyone know why that struct proc is marked as volatile? Can the volatile qualifier safely be removed? 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 freebsd-fs" in the body of the message From owner-freebsd-fs Mon Aug 28 9:43:42 2000 Delivered-To: freebsd-fs@freebsd.org Received: from fledge.watson.org (fledge.watson.org [204.156.12.50]) by hub.freebsd.org (Postfix) with ESMTP id D9CB737B424; Mon, 28 Aug 2000 09:43:39 -0700 (PDT) Received: from fledge.watson.org (robert@fledge.pr.watson.org [192.0.2.3]) by fledge.watson.org (8.9.3/8.9.3) with SMTP id MAA85796; Mon, 28 Aug 2000 12:43:39 -0400 (EDT) (envelope-from robert@fledge.watson.org) Date: Mon, 28 Aug 2000 12:43:39 -0400 (EDT) From: Robert Watson X-Sender: robert@fledge.watson.org To: phk@FreeBSD.org Cc: freebsd-fs@FreeBSD.org Subject: Re: procfs_lookup() and jail interaction In-Reply-To: Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-fs@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.org So I've largely resolved these concerns -- as a synthetic in-memory file system, procfs is not using the name cache -- the issue I'm running into now in procfs is with the open() syscall. Following the p_stuff patches, procfs_getattrt() and so on all return ENOENT. However, an attempt to call open(/proc/1, O_CREAT) results in an EISDIR error, instead of EROFS. I believe this may be a result of that type check happening in vn_open, above the VFS layer, resulting in procfs_* never seeing the request, and thereby revealing the presence of the directory. You can replicate this simply by calling, ``touch /proc/1'' with kern.ps_showallprocs set to 0. It should be the same if you apply the p_stuff.diff patch previously advertised on -security, which cleans up a number of inter-process authorization checks, as well as vaccess(). 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 freebsd-fs" in the body of the message From owner-freebsd-fs Mon Aug 28 10: 0:10 2000 Delivered-To: freebsd-fs@freebsd.org Received: from critter.freebsd.dk (flutter.freebsd.dk [212.242.40.147]) by hub.freebsd.org (Postfix) with ESMTP id F162837B43E; Mon, 28 Aug 2000 10:00:04 -0700 (PDT) Received: from critter (localhost [127.0.0.1]) by critter.freebsd.dk (8.11.0/8.9.3) with ESMTP id e7SH03N11699; Mon, 28 Aug 2000 19:00:03 +0200 (CEST) (envelope-from phk@critter.freebsd.dk) To: Robert Watson Cc: freebsd-fs@FreeBSD.ORG Subject: Re: procfs_lookup() and jail interaction In-Reply-To: Your message of "Mon, 28 Aug 2000 12:43:39 EDT." Date: Mon, 28 Aug 2000 19:00:03 +0200 Message-ID: <11697.967482003@critter> From: Poul-Henning Kamp Sender: owner-freebsd-fs@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.org In message , Robe rt Watson writes: > >So I've largely resolved these concerns -- as a synthetic in-memory file >system, procfs is not using the name cache -- the issue I'm running into >now in procfs is with the open() syscall. Following the p_stuff patches, >procfs_getattrt() and so on all return ENOENT. However, an attempt to >call open(/proc/1, O_CREAT) results in an EISDIR error, instead of EROFS. >I believe this may be a result of that type check happening in vn_open, >above the VFS layer, resulting in procfs_* never seeing the request, and >thereby revealing the presence of the directory. Uhm, isn't a VOP_GETATTR() done to find out what we're fiddling ? How else would it know that it is a directory ? -- Poul-Henning Kamp | UNIX since Zilog Zeus 3.20 phk@FreeBSD.ORG | TCP/IP since RFC 956 FreeBSD coreteam member | BSD since 4.3-tahoe Never attribute to malice what can adequately be explained by incompetence. To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-fs" in the body of the message From owner-freebsd-fs Mon Aug 28 10: 8:56 2000 Delivered-To: freebsd-fs@freebsd.org Received: from fledge.watson.org (fledge.watson.org [204.156.12.50]) by hub.freebsd.org (Postfix) with ESMTP id 08B9E37B422 for ; Mon, 28 Aug 2000 10:08:51 -0700 (PDT) Received: from fledge.watson.org (robert@fledge.pr.watson.org [192.0.2.3]) by fledge.watson.org (8.9.3/8.9.3) with SMTP id NAA85978; Mon, 28 Aug 2000 13:06:17 -0400 (EDT) (envelope-from robert@fledge.watson.org) Date: Mon, 28 Aug 2000 13:06:17 -0400 (EDT) From: Robert Watson X-Sender: robert@fledge.watson.org To: Poul-Henning Kamp Cc: freebsd-fs@FreeBSD.ORG Subject: Re: procfs_lookup() and jail interaction In-Reply-To: <11697.967482003@critter> Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-fs@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.org On Mon, 28 Aug 2000, Poul-Henning Kamp wrote: > In message , Robe > rt Watson writes: > > > >So I've largely resolved these concerns -- as a synthetic in-memory file > >system, procfs is not using the name cache -- the issue I'm running into > >now in procfs is with the open() syscall. Following the p_stuff patches, > >procfs_getattrt() and so on all return ENOENT. However, an attempt to > >call open(/proc/1, O_CREAT) results in an EISDIR error, instead of EROFS. > >I believe this may be a result of that type check happening in vn_open, > >above the VFS layer, resulting in procfs_* never seeing the request, and > >thereby revealing the presence of the directory. > > Uhm, isn't a VOP_GETATTR() done to find out what we're fiddling ? > > How else would it know that it is a directory ? So perhaps I need to do some more tracing to track this down further, but my believe is that procfs_getattr() should not be returning information to the calling process: switch (pfs->pfs_type) { case Proot: case Pcurproc: procp = 0; break; default: procp = PFIND(pfs->pfs_pid); if (procp == 0 || procp->p_cred == NULL || procp->p_ucred == NULL) return (ENOENT); if (p_cansee(ap->a_p, procp, NULL)) return (ENOENT); } However: alsvid:/data/fbsd-commit/src/sys/miscfs/procfs> touch /proc/1 alsvid:/data/fbsd-commit/src/sys/miscfs/procfs> su Password: alsvid# sysctl -w kern.ps_showallprocs=0 kern.ps_showallprocs: 1 -> 0 alsvid# exit alsvid:/data/fbsd-commit/src/sys/miscfs/procfs> touch /proc/1 touch: /proc/1: Is a directory alsvid:/data/fbsd-commit/src/sys/miscfs/procfs> But: alsvid:/data/fbsd-commit/src/sys/miscfs/procfs> ktrace touch /proc/1 touch: /proc/1: Is a directory alsvid:/data/fbsd-commit/src/sys/miscfs/procfs> kdump ... 260 touch CALL stat(0xbfbffc42,0xbfbffaa0) 260 touch NAMI "/proc/1" 260 touch RET stat -1 errno 2 No such file or directory 260 touch CALL open(0xbfbffc42,0x201,0x1b6) 260 touch NAMI "/proc/1" 260 touch RET open -1 errno 21 Is a directory So open() is returning EISDIR. It looks like vn_open looks directory at vp->v_type to determine if it's a directory, not relying on the results of VOP_GETATTR: if ((fmode & O_CREAT) == 0) { mode = 0; if (fmode & (FWRITE | O_TRUNC)) { if (vp->v_type == VDIR) { error = EISDIR; goto bad; } So the check is still happening above the VFS layer. I'll look at the code further this evening. 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 freebsd-fs" in the body of the message From owner-freebsd-fs Mon Aug 28 10:19:52 2000 Delivered-To: freebsd-fs@freebsd.org Received: from critter.freebsd.dk (flutter.freebsd.dk [212.242.40.147]) by hub.freebsd.org (Postfix) with ESMTP id 420A837B424; Mon, 28 Aug 2000 10:19:49 -0700 (PDT) Received: from critter (localhost [127.0.0.1]) by critter.freebsd.dk (8.11.0/8.9.3) with ESMTP id e7SHJmN11834; Mon, 28 Aug 2000 19:19:48 +0200 (CEST) (envelope-from phk@critter.freebsd.dk) To: Robert Watson Cc: freebsd-fs@FreeBSD.ORG Subject: Re: procfs_lookup() and jail interaction In-Reply-To: Your message of "Mon, 28 Aug 2000 13:06:17 EDT." Date: Mon, 28 Aug 2000 19:19:48 +0200 Message-ID: <11832.967483188@critter> From: Poul-Henning Kamp Sender: owner-freebsd-fs@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.org In message , Robe rt Watson writes: >It looks like vn_open looks directory at vp->v_type to determine if it's a >directory, not relying on the results of VOP_GETATTR: > > if ((fmode & O_CREAT) == 0) { > mode = 0; > if (fmode & (FWRITE | O_TRUNC)) { > if (vp->v_type == VDIR) { > error = EISDIR; > goto bad; > } > >So the check is still happening above the VFS layer. > >I'll look at the code further this evening. Hmm, but vn_open needs to find the vnode first, and that means a VOP_LOOKUP into procfs doesn't it ? How else (when the vfscache isn't used) would it know which vnode ? -- Poul-Henning Kamp | UNIX since Zilog Zeus 3.20 phk@FreeBSD.ORG | TCP/IP since RFC 956 FreeBSD coreteam member | BSD since 4.3-tahoe Never attribute to malice what can adequately be explained by incompetence. To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-fs" in the body of the message From owner-freebsd-fs Mon Aug 28 10:38:27 2000 Delivered-To: freebsd-fs@freebsd.org Received: from fledge.watson.org (fledge.watson.org [204.156.12.50]) by hub.freebsd.org (Postfix) with ESMTP id 0A82537B424 for ; Mon, 28 Aug 2000 10:38:25 -0700 (PDT) Received: from fledge.watson.org (robert@fledge.pr.watson.org [192.0.2.3]) by fledge.watson.org (8.9.3/8.9.3) with SMTP id NAA86273; Mon, 28 Aug 2000 13:35:32 -0400 (EDT) (envelope-from robert@fledge.watson.org) Date: Mon, 28 Aug 2000 13:35:32 -0400 (EDT) From: Robert Watson X-Sender: robert@fledge.watson.org To: Poul-Henning Kamp Cc: freebsd-fs@FreeBSD.ORG Subject: Re: procfs_lookup() and jail interaction In-Reply-To: <11832.967483188@critter> Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-fs@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.org On Mon, 28 Aug 2000, Poul-Henning Kamp wrote: > >So the check is still happening above the VFS layer. > > > >I'll look at the code further this evening. > > Hmm, but vn_open needs to find the vnode first, and that means > a VOP_LOOKUP into procfs doesn't it ? How else (when the vfscache > isn't used) would it know which vnode ? Sure enough, procfs_lookup() was missing a call to {PRISON_CHECK(), p_cansee()}, so adding one now gives the right results: alsvid:~> touch /proc/1 touch: /proc/1: Read-only file system I'll update p_stuff, and keep looking for other problems. Thanks, 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 freebsd-fs" in the body of the message From owner-freebsd-fs Mon Aug 28 18:10:12 2000 Delivered-To: freebsd-fs@freebsd.org Received: from relay.butya.kz (butya-gw.butya.kz [212.154.129.94]) by hub.freebsd.org (Postfix) with ESMTP id 5B45A37B43C; Mon, 28 Aug 2000 18:10:07 -0700 (PDT) Received: from bp.dialup.butya.kz (bp.dialup.butya.kz [192.168.1.193]) by relay.butya.kz (Postfix) with ESMTP id BE74F28703; Tue, 29 Aug 2000 08:10:02 +0700 (ALMST) Date: Tue, 29 Aug 2000 08:14:35 +0700 (ALMST) From: Boris Popov To: Robert Watson Cc: phk@FreeBSD.org, freebsd-fs@FreeBSD.org Subject: Re: procfs_lookup() and jail interaction In-Reply-To: Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-fs@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.org On Mon, 28 Aug 2000, Robert Watson wrote: > So I've largely resolved these concerns -- as a synthetic in-memory file > system, procfs is not using the name cache -- the issue I'm running into > now in procfs is with the open() syscall. Following the p_stuff patches, > procfs_getattrt() and so on all return ENOENT. However, an attempt to > call open(/proc/1, O_CREAT) results in an EISDIR error, instead of EROFS. This is because procfs_lookup() function doesn't check for CREATE operation. I'm unsure why this test is omitted because procfs has readonly namespace. The following patch should fix this: --- procfs_vnops.c.old Sun Aug 27 09:43:37 2000 +++ procfs_vnops.c Tue Aug 29 08:00:08 2000 @@ -683,7 +683,8 @@ *vpp = NULL; - if (cnp->cn_nameiop == DELETE || cnp->cn_nameiop == RENAME) + if (cnp->cn_nameiop == DELETE || cnp->cn_nameiop == RENAME || + cnp->cn_nameiop == CREATE) return (EROFS); if (cnp->cn_namelen == 1 && *pname == '.') { -- Boris Popov To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-fs" in the body of the message From owner-freebsd-fs Mon Aug 28 21:33: 9 2000 Delivered-To: freebsd-fs@freebsd.org Received: from mailman.zeta.org.au (mailman.zeta.org.au [203.26.10.16]) by hub.freebsd.org (Postfix) with ESMTP id 5E69937B43C; Mon, 28 Aug 2000 21:33:05 -0700 (PDT) Received: from bde.zeta.org.au (bde.zeta.org.au [203.2.228.102]) by mailman.zeta.org.au (8.8.7/8.8.7) with ESMTP id PAA10224; Tue, 29 Aug 2000 15:33:00 +1100 Date: Tue, 29 Aug 2000 15:32:58 +1100 (EST) From: Bruce Evans X-Sender: bde@besplex.bde.org To: Robert Watson Cc: freebsd-fs@FreeBSD.ORG Subject: Re: volatile struct proc pointer in procfs_vnops.o In-Reply-To: Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-fs@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.org On Mon, 28 Aug 2000, Robert Watson wrote: > I'm getting a couple of warnings compiling procfs_vnops.o with p_cansee() > instead of PRISON_CHECK(), as the struct proc *p is qualified as volatile: > > int pcnt = 0; > volatile struct proc *p = allproc.lh_first; > > ... > ../../miscfs/procfs/procfs_vnops.c: In function `procfs_readdir': > ../../miscfs/procfs/procfs_vnops.c:882: warning: passing arg 2 of > `p_cansee' discards qualifiers from pointer target type > ../../miscfs/procfs/procfs_vnops.c:886: warning: passing arg 2 of > `p_cansee' discards qualifiers from pointer target type > > Does anyone know why that struct proc is marked as volatile? Can the > volatile qualifier safely be removed? I think it can be removed. It seems to be a naive attempt to handle the allproc list changing while we traverse it. However, if the list actually changed, then traversing it might lead to garbage, since the list-changing code doesn't attempt to preserve the integrity of the list. In practice, the list doesn't actually change (e.g., because it is protected by the giant lock in the SMP case), and all other traversals of it (except a clone in linprocfs :-()) depend on this and don't use "volatile". Bruce To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-fs" in the body of the message