{"thread":{"id":"66033","subject":"[PATCH 0/2] Rust hash cleanups","startedAt":"2026-07-19T01:08:54Z","lastAt":"2026-07-19T08:08:02Z","messageCount":4,"participants":["brian m. carlson","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"548614","messageId":"20260719010842.17991-1-sandals@crustytoothpaste.net","threadId":"66033","inReplyTo":null,"subject":"[PATCH 0/2] Rust hash cleanups","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2026-07-19T01:08:40Z","receivedAt":"2026-07-19T01:08:54Z","isPatch":true,"body":"Peff recently sent out a series to fix several memory leaks with our\nhashing code when not using the default block algorithm. This series\nfollows up with a few fixes to our Rust hash code, which calls the C\ncode, to fix various memory problems.\n\nbrian m. carlson (2):\n hash: initialize context before cloning\n rust: discard hash context when finished\n\n src/hash.rs | 13 +++++++++++--\n 1 file changed, 11 insertions(+), 2 deletions(-)\n\n"},{"id":"548615","messageId":"20260719010842.17991-3-sandals@crustytoothpaste.net","threadId":"66033","inReplyTo":"20260719010842.17991-1-sandals@crustytoothpaste.net","subject":"[PATCH 2/2] rust: discard hash context when finished","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2026-07-19T01:08:42Z","receivedAt":"2026-07-19T01:08:54Z","isPatch":true,"body":"When we allocate a context but then abandon it, we never discard it,\nwhich means that the underlying crypto library context may leak. This\ndoesn't happen with our default block code, but it may with OpenSSL.\nNote that we do call git_hash_free, which frees the memory we called\nfrom git_hash_alloc, but doesn't discard the underlying context itself.\n\nThis can be seen with the following command when compiling with OpenSSL\nand running with nightly Rust:\n\n RUSTFLAGS='-Z sanitizer=leak' cargo test\n\nDiscard the context in our context handler. Note that it is fine to do\nso even after finalizing the context, so our final functions which take\nself instead of &mut self will not mishandle memory.\n\nSigned-off-by: brian m. carlson \n---\n src/hash.rs | 8 +++++++-\n 1 file changed, 7 insertions(+), 1 deletion(-)\n\ndiff --git a/src/hash.rs b/src/hash.rs\nindex 4d14e4b4fa..e1f2d31fc3 100644\n--- a/src/hash.rs\n+++ b/src/hash.rs\n@@ -194,7 +194,10 @@ impl Clone for CryptoHasher {\n \n impl Drop for CryptoHasher {\n fn drop(&mut self) {\n- unsafe { c::git_hash_free(self.ctx) };\n+ unsafe {\n+ c::git_hash_discard(self.ctx);\n+ c::git_hash_free(self.ctx);\n+ };\n }\n }\n \n@@ -356,6 +359,7 @@ pub mod c {\n pub fn git_hash_clone(dst: *mut c_void, src: *const c_void);\n pub fn git_hash_update(ctx: *mut c_void, inp: *const c_void, len: usize);\n pub fn git_hash_final(hash: *mut u8, ctx: *mut c_void);\n+ pub fn git_hash_discard(ctx: *mut c_void);\n pub fn git_hash_final_oid(hash: *mut c_void, ctx: *mut c_void);\n }\n }\n@@ -450,6 +454,7 @@ mod tests {\n h.update(&data[2..]);\n \n let h2 = h.clone();\n+ let h3 = h2.clone();\n \n let actual_oid = h.into_oid();\n assert_eq!(**oid, actual_oid);\n@@ -463,6 +468,7 @@ mod tests {\n \n let actual_oid = h.into_oid();\n assert_eq!(**oid, actual_oid);\n+ std::mem::drop(h3);\n }\n }\n }\n"},{"id":"548616","messageId":"20260719010842.17991-2-sandals@crustytoothpaste.net","threadId":"66033","inReplyTo":"20260719010842.17991-1-sandals@crustytoothpaste.net","subject":"[PATCH 1/2] hash: initialize context before cloning","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2026-07-19T01:08:41Z","receivedAt":"2026-07-19T01:08:54Z","isPatch":true,"body":"Our C-based clone helper requires that the context be initialized, but\nwe neglect to do that in our Clone implementation for CryptoHasher.\nThis does not matter when using our default block SHA-256\nimplementation, but it does cause a crash when using OpenSSL as the\nbackend. Fix this by properly initializing the context before cloning\ninto it.\n\nSigned-off-by: brian m. carlson \n---\n src/hash.rs | 5 ++++-\n 1 file changed, 4 insertions(+), 1 deletion(-)\n\ndiff --git a/src/hash.rs b/src/hash.rs\nindex dea2998de4..4d14e4b4fa 100644\n--- a/src/hash.rs\n+++ b/src/hash.rs\n@@ -181,7 +181,10 @@ impl CryptoDigest for CryptoHasher {\n impl Clone for CryptoHasher {\n fn clone(&self) -> Self {\n let ctx = unsafe { c::git_hash_alloc() };\n- unsafe { c::git_hash_clone(ctx, self.ctx) };\n+ unsafe {\n+ c::git_hash_init(ctx, self.algo.hash_algo_ptr());\n+ c::git_hash_clone(ctx, self.ctx)\n+ };\n Self {\n algo: self.algo,\n ctx,\n"},{"id":"548623","messageId":"20260719080754.GA429688@coredump.intra.peff.net","threadId":"66033","inReplyTo":"20260719010842.17991-1-sandals@crustytoothpaste.net","subject":"Re: [PATCH 0/2] Rust hash cleanups","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-07-19T08:07:54Z","receivedAt":"2026-07-19T08:08:02Z","isPatch":true,"body":"On Sun, Jul 19, 2026 at 01:08:40AM +0000, brian m. carlson wrote:\n\n> Peff recently sent out a series to fix several memory leaks with our\n> hashing code when not using the default block algorithm. This series\n> follows up with a few fixes to our Rust hash code, which calls the C\n> code, to fix various memory problems.\n\nBoth of these look good to me (modulo my almost-zero knowledge of the\nRust bits).\n\nI was worried at first that I had introduced new problems with my fixes,\nbut I think these are both pre-existing issues (really just variants of\nthe cleanups I did in the C code).\n\nFor patch 1, an alternative is to switch git_hash_clone() to _not_\nrequire initialization. But it introduces the leak problem in the\nopposite direction. E.g., hashfile_truncate() wants to overwrite\nexisting state, so it would now need to discard() before cloning. I\ndoubt it's worth the effort or risk of regression to save the tiny bit\nof effort spent on a few init-then-overwrite cases.\n\nSo the approach taken here makes sense (and obviously this is just\nfollowing the C code's lead anyway).\n\n-Peff\n"}]}