From: Joey Hess Date: Fri, 27 Aug 2021 17:09:34 +0000 (-0400) Subject: Run cp -a with --no-preserve=xattr, to avoid problems with copied xattrs X-Git-Tag: archive/raspbian/10.20250416-2+rpi1~1^2~87^2~44 X-Git-Url: https://dgit.raspbian.org/?a=commitdiff_plain;h=e17342b2a023d7de6d1a7ae3e4bc746368ca1b63;p=git-annex.git Run cp -a with --no-preserve=xattr, to avoid problems with copied xattrs Including them breaking permissions setting on some NFS servers. Sponsored-by: Dartmouth College's Datalad project --- diff --git a/Build/Configure.hs b/Build/Configure.hs index 5fd172d3db..83ef161ced 100644 --- a/Build/Configure.hs +++ b/Build/Configure.hs @@ -25,6 +25,7 @@ tests = , testCp "cp_p" "-p" , testCp "cp_preserve_timestamps" "--preserve=timestamps" , testCpReflinkAuto + , testCpNoPreserveXattr , TestCase "xargs -0" $ testCmd "xargs_0" "xargs -0 /dev/null" , TestCase "curl" $ testCmd "curl" "curl --version >/dev/null" @@ -62,6 +63,11 @@ testCpReflinkAuto = testCp k "--reflink=auto" where k = "cp_reflink_supported" +testCpNoPreserveXattr :: TestCase +testCpNoPreserveXattr = testCp + "cp_no_preserve_xattr_supported" + "--no-preserve=xattr" + getUpgradeLocation :: Test getUpgradeLocation = do e <- getEnv "UPGRADE_LOCATION" diff --git a/CHANGELOG b/CHANGELOG index 66d577fc53..2c6cdce457 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -19,6 +19,9 @@ git-annex (8.20210804) UNRELEASED; urgency=medium get, drop, move, copy, whereis * Added annex.youtube-dl-command config. This can be used to run some forks of youtube-dl. + * Run cp -a with --no-preserve=xattr, to avoid problems with copied + xattrs, including them breaking permissions setting on some NFS + servers. -- Joey Hess Tue, 03 Aug 2021 12:22:45 -0400 diff --git a/Utility/CopyFile.hs b/Utility/CopyFile.hs index f851326b90..ed2da15e5c 100644 --- a/Utility/CopyFile.hs +++ b/Utility/CopyFile.hs @@ -1,6 +1,6 @@ {- file copying - - - Copyright 2010-2019 Joey Hess + - Copyright 2010-2021 Joey Hess - - License: BSD-2-clause -} @@ -30,6 +30,12 @@ copyMetaDataParams meta = map snd $ filter fst , Param "-p") , (not allmeta && BuildInfo.cp_preserve_timestamps , Param "--preserve=timestamps") + -- cp -a may preserve xattrs that have special meaning, + -- eg to NFS, and have even been observed to prevent later + -- changing the permissions of the file. So prevent preserving + -- xattrs. + , (allmeta && BuildInfo.cp_a && BuildInfo.cp_no_preserve_xattr_supported + , Param "--no-preserve=xattr") ] where allmeta = meta == CopyAllMetaData diff --git a/doc/bugs/__34__357_out_of_984_tests_failed__34___on_NFS_lustre_mount.mdwn b/doc/bugs/__34__357_out_of_984_tests_failed__34___on_NFS_lustre_mount.mdwn index 0d6ff8e6ba..c5c982a2a8 100644 --- a/doc/bugs/__34__357_out_of_984_tests_failed__34___on_NFS_lustre_mount.mdwn +++ b/doc/bugs/__34__357_out_of_984_tests_failed__34___on_NFS_lustre_mount.mdwn @@ -134,3 +134,5 @@ details of the mount with minimal sensoring with XXXs: any ideas Joey? or how to troubleshoot on this beast further? [[!meta title="357 out of 984 tests fail on NFS isilon mount"]] + +> [[fixed|done]] --[[Joey]] diff --git a/doc/bugs/__34__357_out_of_984_tests_failed__34___on_NFS_lustre_mount/comment_11_541e257ea04d6a7647a337998d79232c._comment b/doc/bugs/__34__357_out_of_984_tests_failed__34___on_NFS_lustre_mount/comment_11_541e257ea04d6a7647a337998d79232c._comment new file mode 100644 index 0000000000..9cf2c56637 --- /dev/null +++ b/doc/bugs/__34__357_out_of_984_tests_failed__34___on_NFS_lustre_mount/comment_11_541e257ea04d6a7647a337998d79232c._comment @@ -0,0 +1,44 @@ +[[!comment format=mdwn + username="joey" + subject="""comment 11""" + date="2021-08-27T16:41:45Z" + content=""" +I've made the change to use that option. I think this should clear the test +suite. + +I see that Isilon is a NAS, so some proprietary and broken NFS server +apparently. Kind of explains why cp -a would do this, because I found +threads about cp -a preserving NFS xattrs, and people wanted it to, +presumably for other reasons on less broken NFS servers. + +This does raise the question of what might happen if someone copies +a file themselves with cp -a on this NFS and then git-annex adds the copy. +Probably git-annex would then be unable to remove the write bit +from the annex object file. + +For that matter, the same could happen with ACLs. Eg, I was able to +use setfacl to make this happen: + + joey@darkstar:~>chmod -w foo + joey@darkstar:~>setfacl -m g:nogroup:rw foo + joey@darkstar:~>ls -l foo + -r--rw-r--+ 1 joey joey 0 Aug 27 12:53 foo + joey@darkstar:~>chmod -w foo + chmod: foo: new permissions are r--rw-r--, not r--r--r-- + - exit 1 + joey@darkstar:~>perl -e 'chmod(400)' foo + joey@darkstar:~>ls -l foo + -r--rw-r--+ 1 joey joey 0 Aug 27 12:53 foo + +So git-annex would be unable to clear the write bit, and would not be able +to effectively lock down the file for all users, eg user nobody can write +to the file in the above example. There's probably a way to let user joey +also write to it, but my attempt to do that with an ACL failed. + +Perhaps git-annex should clear ACLs when ingesting (and locking) files. +But perhaps users use ACLs for other purposes that would not prevent +lockdown, and so they should not be cleared. And as far as internal-use NFS +xattrs, it doesn't seem wise for git-annex to try to unset them from files +its ingesting. So I guess I'm going to punt on this broader question, +if users want to use the ACL rope, it's over there.. +"""]]