From owner-svn-src-head@freebsd.org Tue Nov 21 20:06:12 2017 Return-Path: Delivered-To: svn-src-head@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 A2BFED931D9; Tue, 21 Nov 2017 20:06:12 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail104.syd.optusnet.com.au (mail104.syd.optusnet.com.au [211.29.132.246]) by mx1.freebsd.org (Postfix) with ESMTP id 3B57F7DFB3; Tue, 21 Nov 2017 20:06:12 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from [192.168.0.103] (c110-21-101-228.carlnfd1.nsw.optusnet.com.au [110.21.101.228]) by mail104.syd.optusnet.com.au (Postfix) with ESMTPS id 929A84292A3; Wed, 22 Nov 2017 07:05:59 +1100 (AEDT) Date: Wed, 22 Nov 2017 07:05:58 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Bruce Evans cc: Conrad Meyer , src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r326031 - head/sys/fs/msdosfs In-Reply-To: <20171121205309.F1819@besplex.bde.org> Message-ID: <20171122060728.I944@besplex.bde.org> References: <201711202138.vAKLcOxL080933@repo.freebsd.org> <20171121172511.W1210@besplex.bde.org> <20171121205309.F1819@besplex.bde.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.2 cv=bc8baKHB c=1 sm=1 tr=0 a=PalzARQSbocsUSjMRkwAPg==:117 a=PalzARQSbocsUSjMRkwAPg==:17 a=kj9zAlcOel0A:10 a=ykFP3Gf8lMSxWv5rOTIA:9 a=CjuIK1q_8ugA:10 X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.25 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 21 Nov 2017 20:06:12 -0000 On Tue, 21 Nov 2017, Bruce Evans wrote: > On Tue, 21 Nov 2017, Bruce Evans wrote: > >> On Mon, 20 Nov 2017, Conrad Meyer wrote: >> >>> Log: >>> msdosfs(5): Reflect READONLY attribute in file mode >> ... >> This undoes only a small part of READONLY changes in r254627 (which are >> small part of of r253627). I think r254627 got the READONLY semantics >> wrong, but they were almost consistent and well documented in the commit >> log. Now they are more seriously inconsistent. >> ... > PS: I now see a good reason to not change the READONLY attribute using > permissions (doing so is the inconsistent part of r254627), but perhaps > we should complete the undoing of the changes to ignore READONLY for > permissions. > > Under MSDOS or Windows, you have to use attrib to change attributes. > This corresponds to having to use chflags(1) under FreeBSD. Allowing > chmod(1) to change flags together with permissons gives confusing > non-orthogonal behaviour. Only old FreeBSD and old(?) Cygwin have to > do that since they don't have UF_READONLY (or even chflags). > > Example of non-orthogonal behaviour: the fake default permissions are > -r-xr-xr-x and attributes are R. You try to change this using chmod > u+w. This doesn't change the permissions, but it changes the attributes > to 0. Then on rebooting to msdosfs, the R attribute has been lost. It > is safer to require using only chflags(1 or 2) to change attributes. I now see the correct way to fix this. UF_READONLY should be independent of the permissions and act like a partial immutable flag. Changes for this: - back out the curent commit - remove toggling of ATTR_READONLY for setattr of the mode (the neglected change in r254627) - add return (EACCES) in msdosfs_setattr() if (accmode & VMUMBLE) && (dep->de_Attributes & ATTR_READONLY). Clone 3 lines in ffs for this. ffs used to use VWRITE for VMUMBLE in ufs_vaccessx(), but now uses (VMODIFY_PERMS & ~VADMIN_PERMS) in ufs_vaccess(). I think VWRITE is still enough for msdosfs. msdosfs doesn't use V*PERMS anywhere else. ffs also uses VMODIFY_PERMS for the v_type checks. I think they are for ACLs, but msdosfs doesn't support ACLs. Also check that ATTR_READONLY only prevents writing in MSDOS. It could reasonably make more things immutable. In Windows 7, for readonly files, the DEL commands gives "Access is Denied", but rm in Cygwin gives the POSIX behaviour of prompting for removal. We have lots of experience with the permissions in the mode having little to do with the actual permissions. Write bits in them remain set when the file becomes unwritable when immutable flag(s) or certain other flags are set. ACLs also affect accessibilty but not the mode. They just can't change the mode since their effect depends on user ids. The effect of file flags might also depend on user ids. UF_IMMUTABLE happens to deny writability for all users, but it doesn't deny changing itself for root. Note that after the above changes, ATTR_READONLY denies write access even for root, while its old effect on the permissions only denied write access to non-root. This is probably what is wanted. Bruce