fix potential race in updating inode cache
authorJoey Hess <joeyh@joeyh.name>
Tue, 27 Jul 2021 16:29:10 +0000 (12:29 -0400)
committerJoey Hess <joeyh@joeyh.name>
Tue, 27 Jul 2021 16:29:10 +0000 (12:29 -0400)
In Annex.Content, the object file was statted after pointer files were
populated. But if annex.thin is set, once the pointer files are
populated, the object file can potentially be modified via the hard
link. So, it was possible, though seemingly very unlikely, for the inode
of the modified object file to be cached.

Command.Fix and Command.Fsck had similar problems, statting the work
tree files after they were in place. Changed them to stat the temp file
that gets moved into place. This does rely on .git/annex being on the
same filesystem. If it's not, the cached inode will not be the same as
the one that the temp file gets moved to. Result will be that git-annex
will later need to do an expensive verification of the content of the
worktree files. Note that the cross-filesystem move of the temp file
already is a larger amount of extra work, so this seems acceptable.

Sponsored-by: Luke Shumaker on Patreon
Annex/Content.hs
Command/Fix.hs
Command/Fsck.hs
Database/Keys.hs

index 86a3a97998a4c13135089c479e16b1da70aaf64f..50522a20988877f50361a505a76d89be6641029a 100644 (file)
@@ -346,8 +346,11 @@ moveAnnex key af src = ifM (checkSecureHashes' key)
                        fs <- map (`fromTopFilePath` g)
                                <$> Database.Keys.getAssociatedFiles key
                        unless (null fs) $ do
+                               destic <- withTSDelta $
+                                       liftIO . genInodeCache dest
                                ics <- mapM (populatePointerFile (Restage True) key dest) fs
-                               Database.Keys.storeInodeCaches' key [dest] (catMaybes ics)
+                               Database.Keys.addInodeCaches key
+                                       (catMaybes (destic:ics))
                )
        alreadyhave = liftIO $ R.removeLink src
 
index 6a84652ceaf9dfaaa19be76a50e161106d549226..49e5ca97867e883d212dd4a83c15add3e1faf250 100644 (file)
@@ -77,8 +77,8 @@ breakHardLink file key obj = do
                unlessM (checkedCopyFile key obj tmp' mode) $
                        error "unable to break hard link"
                thawContent tmp'
+               Database.Keys.storeInodeCaches key [tmp']
                modifyContent obj $ freezeContent obj
-       Database.Keys.storeInodeCaches key [file]
        next $ return True
 
 makeHardLink :: RawFilePath -> Key -> CommandPerform
index 9a2b2407b57b90d0f99a07be6bd36664afb0c115..7338a362e253edc08f75809925bab83634632208 100644 (file)
@@ -363,7 +363,7 @@ verifyWorkTree key file = do
                                                void $ checkedCopyFile key obj tmp' mode
                                                thawContent tmp'
                                        )
-                       Database.Keys.storeInodeCaches key [file]
+                               Database.Keys.storeInodeCaches key [tmp']
                _ -> return ()
        return True
 
index f79716c4e535b22da39bed0d4e2792485372e719..83a3e7a49e5ecc89c19cccc1995849e55a8a0fde 100644 (file)
@@ -18,7 +18,6 @@ module Database.Keys (
        getAssociatedKey,
        removeAssociatedFile,
        storeInodeCaches,
-       storeInodeCaches',
        addInodeCaches,
        getInodeCaches,
        removeInodeCaches,
@@ -175,11 +174,8 @@ removeAssociatedFile k = runWriterIO . SQL.removeAssociatedFile k
 
 {- Stats the files, and stores their InodeCaches. -}
 storeInodeCaches :: Key -> [RawFilePath] -> Annex ()
-storeInodeCaches k fs = storeInodeCaches' k fs []
-
-storeInodeCaches' :: Key -> [RawFilePath] -> [InodeCache] -> Annex ()
-storeInodeCaches' k fs ics = withTSDelta $ \d ->
-       addInodeCaches k . (++ ics) . catMaybes
+storeInodeCaches k fs = withTSDelta $ \d ->
+       addInodeCaches k . catMaybes
                =<< liftIO (mapM (\f -> genInodeCache f d) fs)
 
 addInodeCaches :: Key -> [InodeCache] -> Annex ()