git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "ノウラ | Flare via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: "ノウラ | Flare" <nouraellm@gmail.com>, "ノウラ | Flare" <nouraellm@gmail.com>
Subject: [PATCH v2] alloc: fix dangling pointer in alloc_state cleanup
Date: Wed, 27 Aug 2025 23:28:32 +0000	[thread overview]
Message-ID: <pull.2040.v2.git.git.1756337312918.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.2040.git.git.1756238268790.gitgitgadget@gmail.com>

From: =?UTF-8?q?=E3=83=8E=E3=82=A6=E3=83=A9=20=7C=20Flare?=
 <nouraellm@gmail.com>

clear_alloc_state() freed all slabs and nulled the slabs pointer but
left slab_alloc, nr, and p unchanged. If the alloc_state is reused,
ALLOC_GROW() can wrongly assume that the slab array is already
allocated because slab_alloc still holds a stale nonzero capacity.
In that case s->slabs remains NULL and the next dereference writes
through a NULL pointer, causing undefined behavior.

To fix this, this patch:

- Renames allocate_alloc_state() → alloc_state_alloc().
- Replaces the “just clear” API with
  alloc_state_free_and_null(struct alloc_state **s_),
  which frees all slabs and the alloc_state itself,
  and then nulls the caller’s pointer.
- Updates call sites to use the new helpers and drops
  redundant FREE_AND_NULL() calls.

This makes the alloc_state lifecycle API harder to misuse,
eliminates stale-capacity state,
and aligns naming with project conventions.

Signed-off-by: ノウラ | Flare <nouraellm@gmail.com>
---
    alloc: fix dangling pointer in alloc_state cleanup

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2040%2Fnouraellm%2Ffix-dangling-pointer-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2040/nouraellm/fix-dangling-pointer-v2
Pull-Request: https://github.com/git/git/pull/2040

Range-diff vs v1:

 1:  643f14e6225 < -:  ----------- reset slab_alloc and state fields in clear_alloc_state()
 -:  ----------- > 1:  0b980850bdf alloc: fix dangling pointer in alloc_state cleanup


 alloc.c  |  9 +++++++--
 alloc.h  |  4 ++--
 object.c | 26 ++++++++++----------------
 3 files changed, 19 insertions(+), 20 deletions(-)

diff --git a/alloc.c b/alloc.c
index 377e80f5dda..c1065551c07 100644
--- a/alloc.c
+++ b/alloc.c
@@ -36,19 +36,24 @@ struct alloc_state {
 	int slab_nr, slab_alloc;
 };
 
-struct alloc_state *allocate_alloc_state(void)
+struct alloc_state *alloc_state_alloc(void)
 {
 	return xcalloc(1, sizeof(struct alloc_state));
 }
 
-void clear_alloc_state(struct alloc_state *s)
+void alloc_state_free_and_null(struct alloc_state **s_)
 {
+	struct alloc_state *s = *s_;
+
+	if (!s_ || !*s_) return;
+
 	while (s->slab_nr > 0) {
 		s->slab_nr--;
 		free(s->slabs[s->slab_nr]);
 	}
 
 	FREE_AND_NULL(s->slabs);
+	FREE_AND_NULL(*s_);
 }
 
 static inline void *alloc_node(struct alloc_state *s, size_t node_size)
diff --git a/alloc.h b/alloc.h
index 3f4a0ad310a..87a47a97095 100644
--- a/alloc.h
+++ b/alloc.h
@@ -14,7 +14,7 @@ void *alloc_commit_node(struct repository *r);
 void *alloc_tag_node(struct repository *r);
 void *alloc_object_node(struct repository *r);
 
-struct alloc_state *allocate_alloc_state(void);
-void clear_alloc_state(struct alloc_state *s);
+struct alloc_state *alloc_state_alloc(void);
+void alloc_state_free_and_null(struct alloc_state **s_);
 
 #endif
diff --git a/object.c b/object.c
index c1553ee4330..986114a6dba 100644
--- a/object.c
+++ b/object.c
@@ -517,12 +517,11 @@ struct parsed_object_pool *parsed_object_pool_new(struct repository *repo)
 	memset(o, 0, sizeof(*o));
 
 	o->repo = repo;
-	o->blob_state = allocate_alloc_state();
-	o->tree_state = allocate_alloc_state();
-	o->commit_state = allocate_alloc_state();
-	o->tag_state = allocate_alloc_state();
-	o->object_state = allocate_alloc_state();
-
+	o->blob_state = alloc_state_alloc();
+	o->tree_state = alloc_state_alloc();
+	o->commit_state = alloc_state_alloc();
+	o->tag_state = alloc_state_alloc();
+	o->object_state = alloc_state_alloc();
 	o->is_shallow = -1;
 	CALLOC_ARRAY(o->shallow_stat, 1);
 
@@ -573,16 +572,11 @@ void parsed_object_pool_clear(struct parsed_object_pool *o)
 	o->buffer_slab = NULL;
 
 	parsed_object_pool_reset_commit_grafts(o);
-	clear_alloc_state(o->blob_state);
-	clear_alloc_state(o->tree_state);
-	clear_alloc_state(o->commit_state);
-	clear_alloc_state(o->tag_state);
-	clear_alloc_state(o->object_state);
+	alloc_state_free_and_null(&o->blob_state);
+	alloc_state_free_and_null(&o->tree_state);
+	alloc_state_free_and_null(&o->commit_state);
+	alloc_state_free_and_null(&o->tag_state);
+	alloc_state_free_and_null(&o->object_state);
 	stat_validity_clear(o->shallow_stat);
-	FREE_AND_NULL(o->blob_state);
-	FREE_AND_NULL(o->tree_state);
-	FREE_AND_NULL(o->commit_state);
-	FREE_AND_NULL(o->tag_state);
-	FREE_AND_NULL(o->object_state);
 	FREE_AND_NULL(o->shallow_stat);
 }

base-commit: f814da676ae46aac5be0a98b99373a76dee6cedb
-- 
gitgitgadget

  parent reply	other threads:[~2025-08-27 23:28 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-26 19:57 [PATCH] reset slab_alloc and state fields in clear_alloc_state() ノウラ | Flare via GitGitGadget
2025-08-27  2:07 ` Junio C Hamano
2025-08-27 23:28 ` ノウラ | Flare via GitGitGadget [this message]
2025-08-28 19:29   ` [PATCH v2] alloc: fix dangling pointer in alloc_state cleanup Torsten Bögershausen
2025-08-28 19:47     ` Junio C Hamano
2025-08-28 20:01       ` Junio C Hamano
2025-08-29 13:00   ` [PATCH v3] " ノウラ | Flare via GitGitGadget
2025-09-03 11:18     ` Jeff King
2025-09-03 21:59       ` Junio C Hamano
2025-09-03 23:17     ` [PATCH v4] " ノウラ | Flare via GitGitGadget
2025-09-04  7:47       ` Junio C Hamano
2025-09-04 13:25         ` ノウラ | Flare
2025-09-04 16:43           ` Junio C Hamano
2025-09-04 17:44       ` [PATCH v5] " ノウラ | Flare via GitGitGadget
2025-09-04 20:25         ` Junio C Hamano
2025-09-04 20:49         ` Jeff King
2025-09-04 22:26           ` Junio C Hamano
2025-09-05  0:02             ` ノウラ | Flare
2025-09-05 13:23               ` Jeff King
2025-09-05 17:27                 ` ノウラ | Flare
2025-09-05  0:07             ` ノウラ | Flare
2025-09-05  0:25               ` ノウラ | Flare
2025-09-05  1:03                 ` ノウラ | Flare
2025-09-05 14:39                 ` Junio C Hamano
2025-09-05 17:47                   ` ノウラ | Flare
2025-09-05 13:15             ` Jeff King
2025-09-05 18:51         ` [PATCH v6] " ノウラ | Flare via GitGitGadget
2025-09-05 19:37           ` Junio C Hamano

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=pull.2040.v2.git.git.1756337312918.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=nouraellm@gmail.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).