From owner-freebsd-standards@FreeBSD.ORG Mon Feb 7 11:01:53 2005 Return-Path: Delivered-To: freebsd-standards@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 3227616A4DF for ; Mon, 7 Feb 2005 11:01:53 +0000 (GMT) Received: from freefall.freebsd.org (freefall.freebsd.org [216.136.204.21]) by mx1.FreeBSD.org (Postfix) with ESMTP id 06D7A43D41 for ; Mon, 7 Feb 2005 11:01:53 +0000 (GMT) (envelope-from owner-bugmaster@freebsd.org) Received: from freefall.freebsd.org (peter@localhost [127.0.0.1]) by freefall.freebsd.org (8.13.1/8.13.1) with ESMTP id j17B1qtv059414 for ; Mon, 7 Feb 2005 11:01:52 GMT (envelope-from owner-bugmaster@freebsd.org) Received: (from peter@localhost) by freefall.freebsd.org (8.13.1/8.13.1/Submit) id j17B1quW059409 for freebsd-standards@freebsd.org; Mon, 7 Feb 2005 11:01:52 GMT (envelope-from owner-bugmaster@freebsd.org) Date: Mon, 7 Feb 2005 11:01:52 GMT Message-Id: <200502071101.j17B1quW059409@freefall.freebsd.org> X-Authentication-Warning: freefall.freebsd.org: peter set sender to owner-bugmaster@freebsd.org using -f From: FreeBSD bugmaster To: freebsd-standards@FreeBSD.org Subject: Current problem reports assigned to you X-BeenThere: freebsd-standards@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Standards compliance List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 07 Feb 2005 11:01:53 -0000 Current FreeBSD problem reports Critical problems Serious problems S Submitted Tracker Resp. Description ------------------------------------------------------------------------------- o [2001/03/05] bin/25542 standards /bin/sh: null char in quoted string p [2002/02/25] standards/35307standards standard include files are not standard c o [2002/12/13] kern/46239 standards posix semaphore implementation errors o [2003/04/21] standards/51209standards [PATCH] add a64l()/l64a/l64a_r functions p [2003/06/05] standards/52972standards /bin/sh arithmetic not POSIX compliant o [2003/06/18] kern/53447 standards poll(2) semantics differ from susV3/POSIX o [2003/07/12] standards/54410standards one-true-awk not POSIX compliant (no exte o [2004/01/01] standards/60772standards _Bool and bool should be unsigned o [2004/11/03] standards/73500standards 'set +o' in /bin/sh does not include unse 9 problems total. Non-critical problems S Submitted Tracker Resp. Description ------------------------------------------------------------------------------- o [2000/09/24] bin/21519 standards sys/dir.h should be deprecated some more o [2001/01/16] bin/24390 standards Replacing old dir-symlinks when using /bi s [2001/01/24] standards/24590standards timezone function not compatible witn Sin s [2001/06/18] kern/28260 standards UIO_MAXIOV needs to be made public p [2001/11/20] standards/32126standards getopt(3) not Unix-98 conformant o [2002/02/27] misc/35381 standards incorrect floating-point display of large s [2002/03/19] standards/36076standards Implementation of POSIX fuser command o [2002/06/14] standards/39256standards [v]snprintf aren't POSIX-conformant for s o [2002/07/09] kern/40378 standards stdlib.h gives needless warnings with -an p [2002/08/12] standards/41576standards POSIX compliance of ln(1) o [2002/10/23] standards/44425standards getcwd() succeeds even if current dir has o [2002/12/09] standards/46119standards Priority problems for SCHED_OTHER using p o [2002/12/23] standards/46504standards Warnings in headers o [2003/06/22] standards/53613standards FreeBSD doesn't define EPROTO o [2003/07/24] standards/54809standards pcvt deficits o [2003/07/25] standards/54833standards more pcvt deficits o [2003/07/25] standards/54839standards pcvt deficits o [2003/07/31] standards/55112standards glob.h, glob_t's gl_pathc should be "size o [2003/09/05] standards/56476standards cd9660 unicode support simple hack o [2003/10/29] standards/58676standards grantpt(3) alters storage used by ptsname p [2003/12/26] standards/60597standards FreeBSD's /usr/include lacks of cpio.h s [2004/02/14] standards/62858standards malloc(0) not C99 compliant p [2004/02/21] standards/63173standards Patch to add getopt_long_only(3) to libc o [2004/03/29] kern/64875 standards [patch] add a system call: fdatasync() o [2004/05/07] standards/66357standards make POSIX conformance problem ('sh -e' & o [2004/05/11] standards/66531standards _gettemp uses a far smaller set of filena o [2004/08/22] standards/70813standards [PATCH] ls not Posix compliant o [2004/08/26] docs/70985 standards [patch] sh(1): incomplete documentation o o [2004/09/22] standards/72006standards floating point formating in non-C locales 29 problems total. From owner-freebsd-standards@FreeBSD.ORG Mon Feb 7 21:35:45 2005 Return-Path: Delivered-To: freebsd-standards@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 8AE8416A4CE for ; Mon, 7 Feb 2005 21:35:45 +0000 (GMT) Received: from electra.cse.Buffalo.EDU (electra.cse.Buffalo.EDU [128.205.32.2]) by mx1.FreeBSD.org (Postfix) with ESMTP id 51D2343D2D for ; Mon, 7 Feb 2005 21:35:44 +0000 (GMT) (envelope-from kensmith@cse.Buffalo.EDU) Received: from electra.cse.Buffalo.EDU (kensmith@localhost [127.0.0.1]) j17LZhu3012824 for ; Mon, 7 Feb 2005 16:35:43 -0500 (EST) Received: (from kensmith@localhost) by electra.cse.Buffalo.EDU (8.12.10/8.12.9/Submit) id j17LZhUH012823 for freebsd-standards@freebsd.org; Mon, 7 Feb 2005 16:35:43 -0500 (EST) Date: Mon, 7 Feb 2005 16:35:43 -0500 From: Ken Smith To: freebsd-standards@freebsd.org Message-ID: <20050207213542.GA11204@electra.cse.Buffalo.EDU> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.4.1i Subject: Fixing exec-modifies-atime X-BeenThere: freebsd-standards@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Standards compliance List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 07 Feb 2005 21:35:45 -0000 Someone recently noted we don't set a file's access time when it gets executed, and that's a violation of standards. It turns out this had been discussed back in 2003 but nothing got done at the time because there were several proposed solutions and each of the solutions had their own set of tradeoffs involved. I made an overly simplistic attempt at fixing this last week and Bruce Evans was quick to point out the list of reasons that fix was incorrect so it got backed out. We have since discussed what had been talked about back in 2003. IMHO the patch tacked on below is a good first step. It fixes the problem in UFS and taking care of the other filesystems that support access time should just be a matter of adding something similar to their setattr function. Does anyone see any problems with this approach, or any major flaws with the implementation in this patch? Thanks. Index: sys/sys/vnode.h =================================================================== RCS file: /home/ncvs/src/sys/sys/vnode.h,v retrieving revision 1.276 diff -u -r1.276 vnode.h --- sys/sys/vnode.h 28 Jan 2005 13:08:21 -0000 1.276 +++ sys/sys/vnode.h 2 Feb 2005 19:51:44 -0000 @@ -262,6 +262,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 6 Feb 2005 22:59:54 -0000 @@ -278,10 +278,11 @@ struct ucred *newcred = NULL, *oldcred; struct uidinfo *euip; register_t *stack_base; - int error, len, i; + int error, len, i, start_write_err = 1; struct image_params image_params, *imgp; - struct vattr attr; + struct vattr atimeattr, attr; int (*img_first)(struct image_params *); + struct mount *mp; struct pargs *oldargs = NULL, *newargs = NULL; struct sigacts *oldsigacts, *newsigacts; #ifdef KTRACE @@ -341,19 +342,27 @@ /* * Translate the file name. namei() returns a vnode pointer - * in ni_vp amoung other things. + * in ni_vp among other things. We don't lock it right away + * because after obtaining the vnode we need to call vn_start_write() + * and having the vnode locked while calling that is a bad idea. */ ndp = &nd; - NDINIT(ndp, LOOKUP, LOCKLEAF | FOLLOW | SAVENAME, - UIO_SYSSPACE, args->fname, td); - + NDINIT(ndp, LOOKUP, FOLLOW | SAVENAME, UIO_SYSSPACE, args->fname, td); mtx_lock(&Giant); interpret: - error = namei(ndp); if (error) goto exec_fail; + /* + * Notify the filesystem of an impending write. We will update + * the file's access time. Lock the vnode afterwards. + * vn_start_write() may block if a filesystem snapshot is in + * progress. + */ + start_write_err = vn_start_write(ndp->ni_vp, &mp, V_WAIT | PCATCH); + vn_lock(ndp->ni_vp, LK_EXCLUSIVE | LK_RETRY, td); + imgp->vp = ndp->ni_vp; /* @@ -432,10 +441,12 @@ mac_copy_vnode_label(ndp->ni_vp->v_label, interplabel); #endif vput(ndp->ni_vp); + if (!start_write_err) + vn_finished_write(mp); vm_object_deallocate(imgp->object); imgp->object = NULL; /* set new name to that of the interpreter */ - NDINIT(ndp, LOOKUP, LOCKLEAF | FOLLOW | SAVENAME, + NDINIT(ndp, LOOKUP, FOLLOW | SAVENAME, UIO_SYSSPACE, imgp->interpreter_name, td); goto interpret; } @@ -672,6 +683,17 @@ 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. + */ + if (!(ndp->ni_vp->v_mount->mnt_flag & (MNT_NOATIME | MNT_RDONLY)) && + !start_write_err) { + VATTR_NULL(&atimeattr); + vfs_timestamp(&atimeattr.va_atime); + 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. @@ -739,6 +761,8 @@ if (interplabel != NULL) mac_vnode_label_free(interplabel); #endif + if (!start_write_err) + vn_finished_write(mp); mtx_unlock(&Giant); exit1(td, W_EXITCODE(0, SIGABRT)); /* NOT REACHED */ @@ -750,6 +774,8 @@ if (interplabel != NULL) mac_vnode_label_free(interplabel); #endif + if (!start_write_err) + vn_finished_write(mp); mtx_unlock(&Giant); return (error); } Index: sys/ufs/ufs/ufs_vnops.c =================================================================== RCS file: /home/ncvs/src/sys/ufs/ufs/ufs_vnops.c,v retrieving revision 1.262 diff -u -r1.262 ufs_vnops.c --- sys/ufs/ufs/ufs_vnops.c 2 Feb 2005 14:21:01 -0000 1.262 +++ sys/ufs/ufs/ufs_vnops.c 5 Feb 2005 18:51:11 -0000 @@ -507,6 +507,20 @@ 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 do get handled by lower levels (e.g. ffs_update()). + */ + if (vap->va_vaflags & VA_EXECVE_ATIME) { + ip->i_flag &= ~IN_ACCESS; + ip->i_flag |= IN_LAZYMOD; + DIP_SET(ip, i_atime, vap->va_atime.tv_sec); + DIP_SET(ip, i_atimensec, vap->va_atime.tv_nsec); + return (0); + } if (ip->i_flags & (IMMUTABLE | APPEND)) return (EPERM); /* -- Ken Smith - From there to here, from here to | kensmith@cse.buffalo.edu there, funny things are everywhere. | - Theodore Geisel | From owner-freebsd-standards@FreeBSD.ORG Tue Feb 8 13:46:21 2005 Return-Path: Delivered-To: freebsd-standards@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id EE92D16A4CE for ; Tue, 8 Feb 2005 13:46:21 +0000 (GMT) Received: from mailout2.pacific.net.au (mailout2.pacific.net.au [61.8.0.85]) by mx1.FreeBSD.org (Postfix) with ESMTP id 59D2D43D48 for ; Tue, 8 Feb 2005 13:46:21 +0000 (GMT) (envelope-from bde@zeta.org.au) Received: from mailproxy1.pacific.net.au (mailproxy1.pacific.net.au [61.8.0.86])j18DkHHn010109; Wed, 9 Feb 2005 00:46:17 +1100 Received: from epsplex.bde.org (katana.zip.com.au [61.8.7.246]) j18DkFVU032064; Wed, 9 Feb 2005 00:46:16 +1100 Date: Wed, 9 Feb 2005 00:46:14 +1100 (EST) From: Bruce Evans X-X-Sender: bde@epsplex.bde.org To: Ken Smith In-Reply-To: <20050207213542.GA11204@electra.cse.Buffalo.EDU> Message-ID: <20050209003105.V4056@epsplex.bde.org> References: <20050207213542.GA11204@electra.cse.Buffalo.EDU> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII cc: freebsd-standards@FreeBSD.org Subject: Re: Fixing exec-modifies-atime X-BeenThere: freebsd-standards@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Standards compliance List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 08 Feb 2005 13:46:22 -0000 On Mon, 7 Feb 2005, Ken Smith wrote: > Does anyone see any problems with this approach, or any major flaws > with the implementation in this patch? I found some new problems (more details in private mail): - using VOP_SETATTR() makes us wait for snapshots to complete, but we shouldn't have to wait for operations that don't really involve writing. The wait may be very long if a snapshot is in progress and affect many processes since execs are fairly common (though perhaps less common than writes). - the implementation makes even bogus execs like exec of "/" wait for snapshots to complete. This is not the serious problem that I thought it was when I wrote the private mail -- the vnode is not locked, so only the process doing the silly exec is affected. Bruce