From: Patrick Steinhardt <ps@pks.im>
To: git@vger.kernel.org
Cc: Taylor Blau <me@ttaylorr.com>, Junio C Hamano <gitster@pobox.com>
Subject: [PATCH 3/4] hash: provide generic wrappers to update hash contexts
Date: Fri, 31 Jan 2025 13:55:30 +0100 [thread overview]
Message-ID: <20250131-b4-pks-hash-context-direct-v1-3-67a6d3f49d6e@pks.im> (raw)
In-Reply-To: <20250131-b4-pks-hash-context-direct-v1-0-67a6d3f49d6e@pks.im>
The hash context is supposed to be updated via the `git_hash_algo`
structure, which contains a list of function pointers to update, clone
or finalize a hashing context. This requires the callers to track which
algorithm was used to initialize the context and continue to use the
exact same algorithm. If they fail to do that correctly, it can happen
that we start to access context state of one hash algorithm with
functions of a different hash algorithm. The result would typically be a
segfault, as could be seen e.g. in the patches part of 98422943f0 (Merge
branch 'ps/weak-sha1-for-tail-sum-fix', 2025-01-01).
The situation was significantly improved starting with 04292c3796
(hash.h: drop unsafe_ function variants, 2025-01-23) and its parent
commits. These refactorings ensure that it is not possible to mix up
safe and unsafe variants of the same hash algorithm anymore. But in
theory, it is still possible to mix up different hash algorithms with
each other, even though this is a lot less likely to happen.
But still, we can do better: instead of asking the caller to remember
the hash algorithm used to initialize a context, we can instead make the
context itself remember which algorithm it has been initialized with. If
we do so, callers can use a set of generic helpers to update the context
and don't need to be aware of the hash algorithm at all anymore.
Adapt the context initialization functions to store the hash algorithm
in the hashing context and introduce these generic helpers. Callers will
be adapted in the subsequent commit.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
hash.h | 21 +++++++++++++++++++++
object-file.c | 6 ++++++
2 files changed, 27 insertions(+)
diff --git a/hash.h b/hash.h
index 42b52c6dae..4367acfec5 100644
--- a/hash.h
+++ b/hash.h
@@ -235,6 +235,7 @@ enum get_oid_result {
/* A suitably aligned type for stack allocations of hash contexts. */
struct git_hash_ctx {
+ const struct git_hash_algo *algop;
union {
git_SHA_CTX sha1;
git_SHA_CTX_unsafe sha1_unsafe;
@@ -296,6 +297,26 @@ struct git_hash_algo {
};
extern const struct git_hash_algo hash_algos[GIT_HASH_NALGOS];
+static inline void git_hash_clone(struct git_hash_ctx *dst, const struct git_hash_ctx *src)
+{
+ src->algop->clone_fn(dst, src);
+}
+
+static inline void git_hash_update(struct git_hash_ctx *ctx, const void *in, size_t len)
+{
+ ctx->algop->update_fn(ctx, in, len);
+}
+
+static inline void git_hash_final(unsigned char *hash, struct git_hash_ctx *ctx)
+{
+ ctx->algop->final_fn(hash, ctx);
+}
+
+static inline void git_hash_final_oid(struct object_id *oid, struct git_hash_ctx *ctx)
+{
+ ctx->algop->final_oid_fn(oid, ctx);
+}
+
/*
* Return a GIT_HASH_* constant based on the name. Returns GIT_HASH_UNKNOWN if
* the name doesn't match a known algorithm.
diff --git a/object-file.c b/object-file.c
index 154bcfce78..b7f2af515f 100644
--- a/object-file.c
+++ b/object-file.c
@@ -88,11 +88,13 @@ static const struct object_id null_oid_sha256 = {
static void git_hash_sha1_init(struct git_hash_ctx *ctx)
{
+ ctx->algop = &hash_algos[GIT_HASH_SHA1];
git_SHA1_Init(&ctx->state.sha1);
}
static void git_hash_sha1_clone(struct git_hash_ctx *dst, const struct git_hash_ctx *src)
{
+ dst->algop = src->algop;
git_SHA1_Clone(&dst->state.sha1, &src->state.sha1);
}
@@ -115,11 +117,13 @@ static void git_hash_sha1_final_oid(struct object_id *oid, struct git_hash_ctx *
static void git_hash_sha1_init_unsafe(struct git_hash_ctx *ctx)
{
+ ctx->algop = unsafe_hash_algo(&hash_algos[GIT_HASH_SHA1]);
git_SHA1_Init_unsafe(&ctx->state.sha1_unsafe);
}
static void git_hash_sha1_clone_unsafe(struct git_hash_ctx *dst, const struct git_hash_ctx *src)
{
+ dst->algop = src->algop;
git_SHA1_Clone_unsafe(&dst->state.sha1_unsafe, &src->state.sha1_unsafe);
}
@@ -143,11 +147,13 @@ static void git_hash_sha1_final_oid_unsafe(struct object_id *oid, struct git_has
static void git_hash_sha256_init(struct git_hash_ctx *ctx)
{
+ ctx->algop = unsafe_hash_algo(&hash_algos[GIT_HASH_SHA256]);
git_SHA256_Init(&ctx->state.sha256);
}
static void git_hash_sha256_clone(struct git_hash_ctx *dst, const struct git_hash_ctx *src)
{
+ dst->algop = src->algop;
git_SHA256_Clone(&dst->state.sha256, &src->state.sha256);
}
--
2.48.1.502.g6dc24dfdaf.dirty
next prev parent reply other threads:[~2025-01-31 12:55 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-01-31 12:55 [PATCH 0/4] hash: introduce generic wrappers to update hash contexts Patrick Steinhardt
2025-01-31 12:55 ` [PATCH 1/4] hash: convert hashing context to a structure Patrick Steinhardt
2025-01-31 12:55 ` [PATCH 2/4] hash: stop typedeffing the hash context Patrick Steinhardt
2025-01-31 12:55 ` Patrick Steinhardt [this message]
2025-01-31 12:55 ` [PATCH 4/4] global: adapt callers to use generic hash context helpers Patrick Steinhardt
2025-01-31 18:16 ` [PATCH 0/4] hash: introduce generic wrappers to update hash contexts Junio C Hamano
2025-02-03 5:42 ` Patrick Steinhardt
2025-02-10 22:55 ` Taylor Blau
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250131-b4-pks-hash-context-direct-v1-3-67a6d3f49d6e@pks.im \
--to=ps@pks.im \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=me@ttaylorr.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).