All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.