From owner-svn-src-all@freebsd.org Mon Dec 7 14:57:56 2015 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id D65D49B7914; Mon, 7 Dec 2015 14:57:56 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail106.syd.optusnet.com.au (mail106.syd.optusnet.com.au [211.29.132.42]) by mx1.freebsd.org (Postfix) with ESMTP id A031B186C; Mon, 7 Dec 2015 14:57:56 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from c211-30-166-197.carlnfd1.nsw.optusnet.com.au (c211-30-166-197.carlnfd1.nsw.optusnet.com.au [211.30.166.197]) by mail106.syd.optusnet.com.au (Postfix) with ESMTPS id EAC5E3C8472; Tue, 8 Dec 2015 01:57:48 +1100 (AEDT) Date: Tue, 8 Dec 2015 01:57:47 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Konstantin Belousov cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r291936 - head/sys/ufs/ufs In-Reply-To: <201512071209.tB7C94hd024620@repo.freebsd.org> Message-ID: <20151208011115.P7632@besplex.bde.org> References: <201512071209.tB7C94hd024620@repo.freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.1 cv=PfoC/XVd c=1 sm=1 tr=0 a=KA6XNC2GZCFrdESI5ZmdjQ==:117 a=PO7r1zJSAAAA:8 a=JzwRw_2MAAAA:8 a=kj9zAlcOel0A:10 a=OX0rTVhBeDaU2BaNTDYA:9 a=CjuIK1q_8ugA:10 X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 07 Dec 2015 14:57:57 -0000 On Mon, 7 Dec 2015, Konstantin Belousov wrote: > Log: > Update ctime when atime or birthtime are updated. > > Cleanup setting of ctime/mtime/birthtime: do not set IN_ACCESS or > IN_UPDATE, then clear them with ufs_itimes(), making transient > (possibly inconsistent) change to the times, and then copy > user-supplied times into the inode. Instead, directly clear IN_ACCESS > or IN_UPDATE when user supplied the time, and copy the value into the > inode. > > Minor inconsistency left is that the inode ctime is updated even when > birthtime update attempt is performed on a UFS1 volume. > > Submitted by: bde > MFC after: 2 weeks Thanks. I forgot to remind yo to make the same change in other file systems. Especially ext2fs. msdosfs already has as much as possible of this. POSIX generally requires even null changes of attributes to update the ctime, so it is not wrong to always update the ctime when the only change is for an unsupported birthtime. This also happens for other file systems that don't support birthtimes. Other file systems might also not support other timestamps. msdosfs doesn't support ctimes so it doesn't need to worry about setting them for null and ignored changes to other timestamps. It ignores attempts to set unsupported times, like ffs does for birthtimes. Some versions of msdosfs support birthtimes, but msdosfs_setattr() doesn't support them. But POSIX also requires omitted changes of attributes to update the ctime. For timestamp attributes, it even requires the security hole of updating the ctime when changing the other times fails (perhaps due to no permission to change them), and the nonsense of updating the ctime when the file doesn't exist. The security hole and the nonsense are because POSIX is missing "successful" in the usual wording "Upon successful completion" in one place. The cases with omitted changes occur mainly for utimensat(). UTIME_OMIT for both times gives 2 omitted changes and not 2 null changes. Clearly the ctime should not be changed when all changes are omitted. But POSIX requires utimensat() to mark the ctime for update on all completions. This gives the security hole but not the nonsense of acting on a nonexistent file in a way in a less unintentional way than the missing "successful". POSIX requires not doing the permissions tests for omitted changes. So 2 omitted changes normally result in success for files that would fail the permissions tests for non-omitted changes. Then the correct "Upon successful completion" wording requires marking the ctime. Some other syscalls like chown() provide another way of omitting changes: Uid (uid_t)-1 means to use the current uid of the file, etc. This is closer to a null change than an omitted change. FressBSD has a different set of bugs in this area. For utimes() and also for utimensat(), it has the undocumented behaviour of turning times with tv_sec == -1 into the equivalent of UTIME_OMIT, the standard utimes() doesn't have such a feature and standard utimensat() is only supposed to have this feature by magic values in tv_nsec. Similarly for chown(). For file times, this is the accidental result of an implementation detail. For chown(), it is more intentional. POSIX chown() has smaller wording bugs and doesn't require the security hole. Most appplications do extra work to avoid null changes. It is unclear if this is allowed. Maybe chown -R root:wheel / is require to update the ctime for all files, not just the ones it makes non-null changes to. This is negatively useful, but not disallowed. Bruce