Date: Sun, 04 May 2003 23:17:23 -0700 From: Kirk McKusick <mckusick@beastie.mckusick.com> To: Bruce Evans <bde@zeta.org.au> Cc: Jens Schweikhardt <schweikh@FreeBSD.org> Subject: Re: Access times on executables (kern/25777) Message-ID: <200305050617.h456HNTh017652@beastie.mckusick.com> In-Reply-To: Your message of "Mon, 05 May 2003 12:42:50 %2B1000." <20030505113734.I6343@gamplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
> Date: Mon, 5 May 2003 12:42:50 +1000 (EST)
> From: Bruce Evans <bde@zeta.org.au>
> X-X-Sender: bde@gamplex.bde.org
> To: Kirk McKusick <mckusick@mckusick.com>
> cc: Jens Schweikhardt <schweikh@FreeBSD.org>, "" <arch@FreeBSD.org>,
> Brian Buhrow <buhrow@lothlorien.nfbcal.org>
> 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 <bde@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.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200305050617.h456HNTh017652>