[Updated] D12428: rust: explain why the current `OwningDirstateMap` is unsound
Alphare (Raphaël Gomès)
phabricator at mercurial-scm.org
Mon Apr 4 20:06:44 UTC 2022
Closed by commit rHGdf581751909e: rust: explain why the current `OwningDirstateMap` is unsound (authored by Alphare).
This revision was automatically updated to reflect the committed changes.
This revision was not accepted when it landed; it landed in state "Needs Review".
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST UPDATE
https://phab.mercurial-scm.org/D12428?vs=32762&id=32768
CHANGES SINCE LAST ACTION
https://phab.mercurial-scm.org/D12428/new/
REVISION DETAIL
https://phab.mercurial-scm.org/D12428
AFFECTED FILES
rust/hg-core/src/dirstate_tree/owning.rs
CHANGE DETAILS
diff --git a/rust/hg-core/src/dirstate_tree/owning.rs b/rust/hg-core/src/dirstate_tree/owning.rs
--- a/rust/hg-core/src/dirstate_tree/owning.rs
+++ b/rust/hg-core/src/dirstate_tree/owning.rs
@@ -2,6 +2,43 @@
use stable_deref_trait::StableDeref;
use std::ops::Deref;
+/*
+// /!\ This is unsound and can cause use after free. It will be fixed in the
+// next patch
+
+// If we change `value` from its current use of `HgPathBuf` to `&HgPath`,
+// nothing here tells that `value` will outlive `OwningDirstateMap`
+pub fn copy_map_insert<'a,'owned>(
+ &'owned mut self,
+ key: &HgPath,
+ value: &'a HgPath,
+) -> Result<Option<HgPathBuf>, DirstateV2ParseError> {
+ // `'local` is smaller than `'a` here
+ let map: &'local mut DirstateMap<'local> = self.get_map_mut();
+ let node: &'local mut Node<'local> = DirstateMap::get_or_insert_node(
+ map.on_disk,
+ &mut map.unreachable_bytes,
+ &mut map.root,
+ &key,
+ WithBasename::to_cow_owned,
+ |_ancestor| {},
+ )?;
+ if node.copy_source.is_none() {
+ map.nodes_with_copy_source_count += 1
+ }
+ Ok(node.copy_source.replace(value.into()).map(Cow::into_owned))
+ // and right here ----------^^^^^^^^^^^^
+ // we are storing `&'a HgPath` in `Node<'local>` which is possible
+ // because to the compiler, `'a` is longer than ``local`.
+ // It is wrong because nothing proves that `&'a HgPath` will outlive `self`.
+}
+
+// All of this is caused by the wrong cast of the DirstateMap pointer that
+// fakes the lifetime of `DirstateMap` and ensures the compiler that it lives
+// as long as `on_disk`, which is only true for its immutable data.
+// This will be fixed in the next commit.
+*/
+
/// Keep a `DirstateMap<'on_disk>` next to the `on_disk` buffer that it
/// borrows.
///
To: Alphare, #hg-reviewers
Cc: mercurial-patches
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mercurial-scm.org/pipermail/mercurial-patches/attachments/20220404/fd226f09/attachment-0002.html>
More information about the Mercurial-patches
mailing list