From owner-freebsd-arch@FreeBSD.ORG Sun May 4 23:17:25 2003 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 8EB9237B401; Sun, 4 May 2003 23:17:25 -0700 (PDT) Received: from beastie.mckusick.com (beastie.mckusick.com [209.31.233.184]) by mx1.FreeBSD.org (Postfix) with ESMTP id EF16F43FAF; Sun, 4 May 2003 23:17:24 -0700 (PDT) (envelope-from mckusick@beastie.mckusick.com) Received: from beastie.mckusick.com (localhost [127.0.0.1]) by beastie.mckusick.com (8.12.8/8.12.3) with ESMTP id h456HNTh017652; Sun, 4 May 2003 23:17:23 -0700 (PDT) (envelope-from mckusick@beastie.mckusick.com) Message-Id: <200305050617.h456HNTh017652@beastie.mckusick.com> To: Bruce Evans In-Reply-To: Your message of "Mon, 05 May 2003 12:42:50 +1000." <20030505113734.I6343@gamplex.bde.org> Date: Sun, 04 May 2003 23:17:23 -0700 From: Kirk McKusick cc: arch@FreeBSD.org cc: Brian Buhrow cc: Jens Schweikhardt Subject: Re: Access times on executables (kern/25777) X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 05 May 2003 06:17:25 -0000 > Date: Mon, 5 May 2003 12:42:50 +1000 (EST) > From: Bruce Evans > X-X-Sender: bde@gamplex.bde.org > To: Kirk McKusick > cc: Jens Schweikhardt , "" , > Brian Buhrow > Subject: Re: Access times on executables (kern/25777) > In-Reply-To: <200305042351.h44Np2Th017215@beastie.mckusick.com> > X-ASK-Info: Whitelist match > > On Sun, 4 May 2003, Kirk McKusick wrote: > > > From: Bruce Evans > > ... > > This doesn't work unless the user has permission to change the atime > > using utimes(2) with a non-NULL times pointer. > > ... > > OK, so how about instead of use VOP_SETATTR, we just try to VOP_READ > > one byte of data. It will run with the speed of read (and indeed since > > we just mapped in the header above, the data should be in the cache). > > It has the benefit of speed and not requiring the user to own the file. > > It has the drawback of requiring that the file be readable though most > > executables are set to be readable. > > It would work better than that. VOP_READ() works right here since the > permissions checks are done at the vfs level at open(2) time, not on every > VOP_READ() or read(2). > > I think VOP_SETATTR() should work similarly: do permissions checks at > the vfs level using VOP_ACCESS() instead of in every file system, so > that there is less duplicated code and the permissions checks don't > get in the way when you don't want them. This change is now easy for > at least setting times. In Lite2, ufs_setattr() had to dereference > `ip' for its permissions check for times, but this detail is now pushed > down to VOP_ACCESS() using the VADMIN flag. There is a new reference > to `ip' for SF_SNAPSHOT, but this is unrelated to normal permissions > except it can cause EPERM to be returned for reasons not documented > in utimes.2. > > Bruce It will work right on the local filesystem, but I think that it would still be broken on NFS since NFS does permission checking on every RPC. Still, I think that VOP_READ is likely to be more efficient and work in a higher percentage of the times (always on the local filesystem and most of the time on NFS). Thus, I enclose my (new) proposed change below. Kirk McKusick =-=-=-=-=-=-= Index: sys/kern_exec.c =================================================================== RCS file: /usr/ncvs/src/sys/kern/kern_exec.c,v retrieving revision 1.218 diff -c -r1.218 kern_exec.c *** kern_exec.c 1 Apr 2003 01:26:20 -0000 1.218 --- kern_exec.c 5 May 2003 06:15:21 -0000 *************** *** 598,603 **** --- 598,626 ---- exec_setregs(td, imgp->entry_addr, (u_long)(uintptr_t)stack_base, imgp->ps_strings); + /* + * At this point, it counts as an access. + * Ensure that atime gets updated if needed. + */ + if (!(textvp->v_mount->mnt_flag & MNT_NOATIME)) { + struct uio auio; + struct iovec aiov; + char ch; + + auio.uio_iov = &aiov; + auio.uio_iovcnt = 1; + aiov.iov_base = &ch; + aiov.iov_len = 1; + auio.uio_resid = 1; + auio.uio_offset = 0; + auio.uio_segflg = UIO_SYSSPACE; + auio.uio_rw = UIO_READ; + auio.uio_td = td; + vn_lock(textvp, LK_EXCLUSIVE | LK_RETRY, td); + (void) VOP_READ(textvp, &auio, 0, p->p_ucred); + VOP_UNLOCK(textvp, 0, td); + } + done1: /* * Free any resources malloc'd earlier that we didn't use.