Access times on executables (kern/25777)

[Kirk McKusick]

> Date: Mon, 5 May 2003 12:42:50 +1000 (EST)
> From: Bruce Evans <bde at zeta.org.au>
> X-X-Sender: bde at gamplex.bde.org
> To: Kirk McKusick <mckusick at mckusick.com>
> cc: Jens Schweikhardt <schweikh at FreeBSD.org>, "" <arch at FreeBSD.org>,
> 	Brian Buhrow <buhrow at lothlorien.nfbcal.org>
> Subject: Re: Access times on executables (kern/25777) 
> In-Reply-To: <200305042351.h44Np2Th017215 at beastie.mckusick.com>
> X-ASK-Info: Whitelist match
> 
> On Sun, 4 May 2003, Kirk McKusick wrote:
> 
> > 	From: Bruce Evans <bde at zeta.org.au>
> > ...
> > 	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.