fix potential race in updating inode cache

Some uses of linkFromAnnex are inside replaceWorkTreeFile, which was
already safe, but others use it directly on the work tree file, which
was race-prone. Eg, if the work tree file was first removed, then
linkFromAnnex called to populate it, the user could have re-written it in
the interim.

This came to light during an audit of all calls of addInodeCaches,
looking for such races. All the other uses of it seem ok.

Sponsored-by: Brett Eisenberg on Patreon

diff --git a/Annex/Content.hs b/Annex/Content.hs
index 50522a20988877f50361a505a76d89be6641029a..3a4759a22b514f2a1585f260044f960ef2019c03 100644 (file)
--- a/Annex/Content.hs
+++ b/Annex/Content.hs
@@ -30,6 +30,7 @@ module Annex.Content (
        populatePointerFile,
        linkToAnnex,
        linkFromAnnex,
+       linkFromAnnex',
        LinkAnnexResult(..),
        unlinkAnnex,
        checkedCopyFile,
@@ -78,6 +79,7 @@ import Annex.Link
 import Annex.LockPool
 import Annex.UUID
 import Annex.InodeSentinal
+import Annex.ReplaceFile
 import Annex.AdjustedBranch (adjustedBranchRefresh)
 import Messages.Progress
 import Types.Remote (RetrievalSecurityPolicy(..))
@@ -382,9 +384,23 @@ linkToAnnex key src srcic = ifM (checkSecureHashes' key)
        , return LinkAnnexFailed
        )
 
-{- Makes a destination file be a link or copy from the annex object. -}
+{- Makes a destination file be a link or copy from the annex object.
+ -
+ - linkAnnex stats the file after copying it to add to the inode
+ - cache. But dest may be a file in the working tree, which could
+ - get modified immediately after being populated. To avoid such a
+ - race, call linkAnnex on a temporary file and move it into place
+ - afterwards. Note that a consequence of this is that, if the file
+ - already exists, it will be overwritten.
+ -}
 linkFromAnnex :: Key -> RawFilePath -> Maybe FileMode -> Annex LinkAnnexResult
-linkFromAnnex key dest destmode = do
+linkFromAnnex key dest destmode =
+       replaceFile (const noop) (fromRawFilePath dest) $ \tmp ->
+               linkFromAnnex' key (toRawFilePath tmp) destmode
+
+{- This is only safe to use when dest is not a worktree file. -}
+linkFromAnnex' :: Key -> RawFilePath -> Maybe FileMode -> Annex LinkAnnexResult
+linkFromAnnex' key dest destmode = do
        src <- calcRepo (gitAnnexLocation key)
        srcic <- withTSDelta (liftIO . genInodeCache src)
        linkAnnex From key src srcic dest destmode

diff --git a/Annex/WorkTree.hs b/Annex/WorkTree.hs
index 9a3dd50777ef63a18e2b46d7d6e4f90375c1c466..60b460e8130744a804d6e6746b147b4e6a4944e5 100644 (file)
--- a/Annex/WorkTree.hs
+++ b/Annex/WorkTree.hs
@@ -131,7 +131,7 @@ scanAnnexedFiles initscan = do
                                                fileMode <$> R.getFileStatus f
                                        ic <- replaceWorkTreeFile (fromRawFilePath f) $ \tmp -> do
                                                let tmp' = toRawFilePath tmp
-                                               linkFromAnnex k tmp' destmode >>= \case
+                                               linkFromAnnex' k tmp' destmode >>= \case
                                                        LinkAnnexOk -> 
                                                                withTSDelta (liftIO . genInodeCache tmp')
                                                        LinkAnnexNoop -> return Nothing

diff --git a/Command/Fix.hs b/Command/Fix.hs
index 49e5ca97867e883d212dd4a83c15add3e1faf250..da2f5dd4ce36718347b18fa840f1f47a24414729 100644 (file)
--- a/Command/Fix.hs
+++ b/Command/Fix.hs
@@ -85,7 +85,7 @@ makeHardLink :: RawFilePath -> Key -> CommandPerform
 makeHardLink file key = do
        replaceWorkTreeFile (fromRawFilePath file) $ \tmp -> do
                mode <- liftIO $ catchMaybeIO $ fileMode <$> R.getFileStatus file
-               linkFromAnnex key (toRawFilePath tmp) mode >>= \case
+               linkFromAnnex' key (toRawFilePath tmp) mode >>= \case
                        LinkAnnexFailed -> error "unable to make hard link"
                        _ -> noop
        next $ return True

diff --git a/Command/Fsck.hs b/Command/Fsck.hs
index 7338a362e253edc08f75809925bab83634632208..0d0cf11e05f88909d9c022f3c4e4335c0920c5ae 100644 (file)
--- a/Command/Fsck.hs
+++ b/Command/Fsck.hs
@@ -357,7 +357,7 @@ verifyWorkTree key file = do
                                let tmp' = toRawFilePath tmp
                                mode <- liftIO $ catchMaybeIO $ fileMode <$> R.getFileStatus file
                                ifM (annexThin <$> Annex.getGitConfig)
-                                       ( void $ linkFromAnnex key tmp' mode
+                                       ( void $ linkFromAnnex' key tmp' mode
                                        , do
                                                obj <- calcRepo (gitAnnexLocation key)
                                                void $ checkedCopyFile key obj tmp' mode

diff --git a/Command/Unlock.hs b/Command/Unlock.hs
index cfe49f19eee0d53bb6b72060f2a4ff16effe48b1..0d3fedd4fb48b6b48d6ba5c70c96620e2eb4f51a 100644 (file)
--- a/Command/Unlock.hs
+++ b/Command/Unlock.hs
@@ -50,7 +50,7 @@ perform dest key = do
        replaceWorkTreeFile (fromRawFilePath dest) $ \tmp ->
                ifM (inAnnex key)
                        ( do
-                               r <- linkFromAnnex key (toRawFilePath tmp) destmode
+                               r <- linkFromAnnex' key (toRawFilePath tmp) destmode
                                case r of
                                        LinkAnnexOk -> return ()
                                        LinkAnnexNoop -> return ()