From owner-freebsd-arch@FreeBSD.ORG Thu May 26 20:24:26 2005 Return-Path: X-Original-To: freebsd-arch@freebsd.org 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 45D5516A41C for ; Thu, 26 May 2005 20:24:26 +0000 (GMT) (envelope-from kensmith@cse.Buffalo.EDU) Received: from opus.cse.buffalo.edu (opus.cse.Buffalo.EDU [128.205.32.4]) by mx1.FreeBSD.org (Postfix) with ESMTP id EA90543D1F for ; Thu, 26 May 2005 20:24:25 +0000 (GMT) (envelope-from kensmith@cse.Buffalo.EDU) Received: from opus.cse.buffalo.edu (opus.cse.buffalo.edu [128.205.32.4]) by opus.cse.buffalo.edu (8.13.3/8.12.10) with ESMTP id j4QKOPHE083084 for ; Thu, 26 May 2005 16:24:25 -0400 (EDT) From: Ken Smith To: freebsd-arch@freebsd.org Content-Type: text/plain Organization: U. Buffalo CSE Department Date: Thu, 26 May 2005 16:24:25 -0400 Message-Id: <1117139065.82793.20.camel@opus.cse.buffalo.edu> Mime-Version: 1.0 X-Mailer: Evolution 2.2.2 FreeBSD GNOME Team Port Content-Transfer-Encoding: 7bit Subject: Modifying file access time upon exec... X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 26 May 2005 20:24:26 -0000 I started working on this many months ago and just as it seemed like we got to a point the people involved (mostly Bruce Evans...) were satisfied with the proposed solution I got ... sidetracked... The issue is that the standards say a file's access time should be modified when it gets executed. We don't do that. This patch takes care of it by adding a new vnode flag that is used by the kernel to indicate that it has blessed an update of a file's access time. The flag gets set at the point the file get executed and the individual filesystems are expected to handle the details. Some don't store access times so it's a non-issue for them. UFS needs to handle it the same way as it would modifying the access time for a read - it shouldn't block, needs to be careful about whether a snapshot is in progress, bypasses most security checks, etc. This patch is mostly Bruce's work. The final version of what he sent me had more changes in it that weren't directly related to fixing the atime issue that I still need to look over and understand better... :-) Any thoughts before I commit it? The patch itself is pretty small. But given the sections of code it's mucking with combined with it adding a little 'nit' filesystem implementers should be aware of I wanted to run it by as many clueful eyes as possible before doing the final commit. Thanks. Index: sys/kern/kern_exec.c =================================================================== RCS file: /home/ncvs/src/sys/kern/kern_exec.c,v retrieving revision 1.271 diff -u -r1.271 kern_exec.c --- sys/kern/kern_exec.c 3 May 2005 16:24:59 -0000 1.271 +++ sys/kern/kern_exec.c 26 May 2005 19:11:04 -0000 @@ -284,7 +284,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; @@ -695,6 +695,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 implemented by the underlying filesystem in the same way as + * access timestamps for a VOP_READ() because we want to avoid + * blocking and/or I/O, and have not called vn_start_write(). + */ + if ((imgp->vp->v_mount->mnt_flag & (MNT_NOATIME | MNT_RDONLY)) == 0) { + VATTR_NULL(&atimeattr); + atimeattr.va_vaflags |= VA_EXECVE_ATIME; + (void)VOP_SETATTR(img->vp, &atimeattr, td->td_ucred, td); + } + done1: /* * Free any resources malloc'd earlier that we didn't use. Index: sys/sys/vnode.h =================================================================== RCS file: /home/ncvs/src/sys/sys/vnode.h,v retrieving revision 1.299 diff -u -r1.299 vnode.h --- sys/sys/vnode.h 27 Apr 2005 09:18:10 -0000 1.299 +++ sys/sys/vnode.h 26 May 2005 15:23:37 -0000 @@ -284,6 +284,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/ufs/ufs/ufs_vnops.c =================================================================== RCS file: /home/ncvs/src/sys/ufs/ufs/ufs_vnops.c,v retrieving revision 1.269 diff -u -r1.269 ufs_vnops.c --- sys/ufs/ufs/ufs_vnops.c 18 May 2005 22:18:21 -0000 1.269 +++ sys/ufs/ufs/ufs_vnops.c 26 May 2005 19:31:59 -0000 @@ -443,6 +443,20 @@ ((int)vap->va_bytes != VNOVAL) || (vap->va_gen != VNOVAL)) { return (EINVAL); } + /* + * Update the file's 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 other levels (e.g., ffs_update()). + */ + if (vap->va_vaflags & VA_EXECVE_ATIME) { + ip->i_flag |= IN_ACCESS; + return (0); + } + /* + * Go through the fields and update iff not VNOVAL. + */ if (vap->va_flags != VNOVAL) { if (vp->v_mount->mnt_flag & MNT_RDONLY) return (EROFS); @@ -493,9 +507,6 @@ } if (ip->i_flags & (IMMUTABLE | APPEND)) return (EPERM); - /* - * Go through the fields and update iff not VNOVAL. - */ if (vap->va_uid != (uid_t)VNOVAL || vap->va_gid != (gid_t)VNOVAL) { if (vp->v_mount->mnt_flag & MNT_RDONLY) return (EROFS); -- Ken Smith - From there to here, from here to | kensmith@cse.buffalo.edu there, funny things are everywhere. | - Theodore Geisel |