Updating atime during exec, attempt #3...

[Ken Smith]

Ok, here is attempt #3 at fixing the exec-should-update-atime problem.
This one is similar to the one I sent a week or so ago in that it is
using a special flag in the vattr structure and VOP_SETATTR() to do the
work.  But this one avoids locking the filesystem for a write (using
vn_start_write()) by letting the filesystem defer the actual update
of atime.  The idea is for the filesystem to handle the update in
roughly the same way as it would for a read, though it should try to
minimize the impact.  UFS is used here as an example - the IN_ATIME
flag gets set (which will result in atime being updated next time the
filesystem gets sync-ed) just like what would happen during a read but
we don't actually do the I/O involved in doing a read.

A filesystem can just totally ignore this extra flag.  Some filesystems
don't support storing something that represents atime.  Other filesystems
will have needed to do a read to support accessing the executable anyway
(NFS appears to do that - execve is using mmap to access the executable
files which is why this bug exists but the mechanism used by NFS to
provide access to the file appears to use the same basic thing as reads).

Does anyone see any problems with this one?

Thanks...

Index: sys/sys/vnode.h
===================================================================
RCS file: /home/ncvs/src/sys/sys/vnode.h,v
retrieving revision 1.280
diff -u -r1.280 vnode.h
--- sys/sys/vnode.h	10 Feb 2005 12:28:58 -0000	1.280
+++ sys/sys/vnode.h	14 Feb 2005 15:30:40 -0000
@@ -255,6 +255,7 @@
  */
 #define	VA_UTIMES_NULL	0x01		/* utimes argument was NULL */
 #define	VA_EXCLUSIVE	0x02		/* exclusive create request */
+#define	VA_EXECVE_ATIME	0x04		/* setting atime for execve */
 
 /*
  * Flags for ioflag. (high 16 bits used to ask for read-ahead and
Index: sys/kern/kern_exec.c
===================================================================
RCS file: /home/ncvs/src/sys/kern/kern_exec.c,v
retrieving revision 1.265
diff -u -r1.265 kern_exec.c
--- sys/kern/kern_exec.c	29 Jan 2005 23:51:05 -0000	1.265
+++ sys/kern/kern_exec.c	14 Feb 2005 15:17:33 -0000
@@ -280,7 +280,7 @@
 	register_t *stack_base;
 	int error, len, i;
 	struct image_params image_params, *imgp;
-	struct vattr attr;
+	struct vattr atimeattr, attr;
 	int (*img_first)(struct image_params *);
 	struct pargs *oldargs = NULL, *newargs = NULL;
 	struct sigacts *oldsigacts, *newsigacts;
@@ -672,6 +672,18 @@
 		exec_setregs(td, imgp->entry_addr,
 		    (u_long)(uintptr_t)stack_base, imgp->ps_strings);
 
+	/*
+	 * Here we should update the access time of the file.  This must
+	 * be handled by the underlying filesystem in the same way that
+	 * the atime gets updated for a VOP_READ() because we have not
+	 * called vn_start_write().
+	 */
+	if (!(ndp->ni_vp->v_mount->mnt_flag & (MNT_NOATIME | MNT_RDONLY))) {
+		VATTR_NULL(&atimeattr);
+		atimeattr.va_vaflags |= VA_EXECVE_ATIME;
+		(void) VOP_SETATTR(ndp->ni_vp, &atimeattr, td->td_ucred, td);
+	}
+
 done1:
 	/*
 	 * Free any resources malloc'd earlier that we didn't use.
Index: sys/ufs/ufs/ufs_vnops.c
===================================================================
RCS file: /home/ncvs/src/sys/ufs/ufs/ufs_vnops.c,v
retrieving revision 1.264
diff -u -r1.264 ufs_vnops.c
--- sys/ufs/ufs/ufs_vnops.c	8 Feb 2005 21:31:10 -0000	1.264
+++ sys/ufs/ufs/ufs_vnops.c	14 Feb 2005 15:19:26 -0000
@@ -511,6 +511,17 @@
 		if (vap->va_flags & (IMMUTABLE | APPEND))
 			return (0);
 	}
+	/*
+	 * Update the file access time when it has been executed.  We are
+	 * doing this here to specifically avoid some of the checks done
+	 * below -- this operation is done by request of the kernel and
+	 * should bypass some security checks.  Things like read-only
+	 * checks get handled by lower levels (e.g., ffs_update()).
+	 */
+	if (vap->va_vaflags & VA_EXECVE_ATIME) {
+		ip->i_flag |= IN_ACCESS;
+		return (0);
+	}
 	if (ip->i_flags & (IMMUTABLE | APPEND))
 		return (EPERM);
 	/*


-- 
						Ken Smith
- From there to here, from here to      |       kensmith at cse.buffalo.edu
  there, funny things are everywhere.   |
                      - Theodore Geisel |