[PATCH] reftable/writer: fix memory leak when `padded_write()` fails

Lidong Yan via GitGitGadget

author

2025-05-10 03:50 UTC #0 lore raw

PATCH 1/1 reftable/writer: fix memory leak when `padded_write()` fails

In reftable/writer.c:padded_write(), if w->writer failed, zeroed allocated in reftable_calloc will leak. w->writer could be reftable_write_data in reftable/stack.c, and could fail due to some write error. Simply add reftable_free(zeroed) will solve this problem.

Signed-off-by Lidong Yan <[email protected]>

---
reftable/writer: fix memory leak when padded_write() fails

In reftable/writer.c:padded_write(), if w->writer failed, zeroed allocated in reftable_calloc will leak. w->writer could be reftable_write_data in reftable/stack.c, and could fail due to some write error. Simply add reftable_free(zeroed) will solve this problem.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1958%2Fbrandb97%2Ffix-reftable-padded-write-leak-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1958/brandb97/fix-reftable-padded-write-leak-v1
Pull-Request: https://github.com/git/git/pull/1958

 reftable/writer.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

reftable/writer.c +3 -1

index cb16f71be49..3ceb3742888 100644
@@ -57,8 +57,10 @@ static int padded_write(struct reftable_writer *w, uint8_t *data, size_t len,
 			return -1;
 
 		n = w->write(w->write_arg, zeroed, w->pending_padding);
-		if (n < 0)
+		if (n < 0) {
+			reftable_free(zeroed);
 			return n;
+		}
 
 		w->pending_padding = 0;
 		reftable_free(zeroed);

base-commit: 6f84262c44a89851c3ae5a6e4c1a9d06b2068d75

Signature · 1 line

gitgitgadget

Lidong Yan via GitGitGadget

author

2025-05-11 16:16 UTC #1 lore raw

PATCH v2 1/1 reftable/writer: fix memory leak if write fails

From: Lidong Yan <[email protected]>

In reftable/writer.c:padded_write(), if w->writer failed, zeroed allocated in reftable_calloc will leak. w->writer could be reftable_write_data in reftable/stack.c, and could fail due to some write error. Simply add reftable_free(zeroed) will solve this problem.

In reftable/writer.c:writer_index_hash(), if reftable_buf_add failed, key allocated by reftable_malloc will not be insert into obj_index_tree thus leaks. Simple add reftable_free(key) will solve this problem.

Signed-off-by Lidong Yan <[email protected]>

---
reftable/writer: fix memory leak when padded_write() fails

In reftable/writer.c:padded_write(), if w->writer failed, zeroed allocated in reftable_calloc will leak. w->writer could be reftable_write_data in reftable/stack.c, and could fail due to some write error. Simply add reftable_free(zeroed) will solve this problem.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1958%2Fbrandb97%2Ffix-reftable-padded-write-leak-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1958/brandb97/fix-reftable-padded-write-leak-v2
Pull-Request: https://github.com/git/git/pull/1958

Range-diff vs v1:

1: 2023d6791ef ! 1: 9329ae2d478 reftable/writer: fix memory leak when padded_write() fails
@@ Metadata
Author: Lidong Yan <[email protected]>

## Commit message ##
- reftable/writer: fix memory leak when padded_write() fails
+ reftable/writer: fix memory leak if write fails

In reftable/writer.c:padded_write(), if w->writer failed, zeroed allocated in reftable_calloc will leak. w->writer could be
@@ Commit message
some write error. Simply add reftable_free(zeroed) will solve this problem.

+ In reftable/writer.c:writer_index_hash(), if reftable_buf_add failed,
+ key allocated by reftable_malloc will not be insert into obj_index_tree
+ thus leaks. Simple add reftable_free(key) will solve this problem.
+

Signed-off-by Lidong Yan <[email protected]>

## reftable/writer.c ##
@@ reftable/writer.c: static int padded_write(struct reftable_writer *w, uint8_t *d

w->pending_padding = 0;
reftable_free(zeroed);
+@@ reftable/writer.c: static int writer_index_hash(struct reftable_writer *w, struct reftable_buf *has +
+ reftable_buf_reset(&key->hash);
+ err = reftable_buf_add(&key->hash, hash->buf, hash->len);
+- if (err < 0)
++ if (err < 0) {
++ reftable_free(key);
+ return err;
++ }
+ tree_insert(&w->obj_index_tree, key,
+ &obj_index_tree_node_compare);
+ } else {

 reftable/writer.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

reftable/writer.c +6 -2

index cb16f71be49..3b4ebdd6dce 100644
@@ -57,8 +57,10 @@ static int padded_write(struct reftable_writer *w, uint8_t *data, size_t len,
 			return -1;
 
 		n = w->write(w->write_arg, zeroed, w->pending_padding);
-		if (n < 0)
+		if (n < 0) {
+			reftable_free(zeroed);
 			return n;
+		}
 
 		w->pending_padding = 0;
 		reftable_free(zeroed);

@@ -256,8 +258,10 @@ static int writer_index_hash(struct reftable_writer *w, struct reftable_buf *has
 
 		reftable_buf_reset(&key->hash);
 		err = reftable_buf_add(&key->hash, hash->buf, hash->len);
-		if (err < 0)
+		if (err < 0) {
+			reftable_free(key);
 			return err;
+		}
 		tree_insert(&w->obj_index_tree, key,
 			    &obj_index_tree_node_compare);
 	} else {

base-commit: 6f84262c44a89851c3ae5a6e4c1a9d06b2068d75

Signature · 1 line

gitgitgadget

Patrick Steinhardt

2025-05-12 08:25 UTC #2 lore raw

Re: [PATCH v2] reftable/writer: fix memory leak if write fails

Quoted 11 lines · Lidong Yan via GitGitGadget

From: Lidong Yan <[email protected]>
In reftable/writer.c:padded_write(), if w->writer failed, zeroed allocated in reftable_calloc will leak. w->writer could be reftable_write_data in reftable/stack.c, and could fail due to some write error. Simply add reftable_free(zeroed) will solve this problem.
In reftable/writer.c:writer_index_hash(), if reftable_buf_add failed, key allocated by reftable_malloc will not be insert into obj_index_tree thus leaks. Simple add reftable_free(key) will solve this problem.

Nit: I think it would be sensible to split these up into two commits, as they touch different areas of the code.

Quoted 6 lines

reftable/writer.c +0 -0

index cb16f71be49..3b4ebdd6dce 100644
@@ -57,8 +57,10 @@ static int padded_write(struct reftable_writer *w, uint8_t *data, size_t len,
 			return -1;

reftable/writer.c static int padded_write(struct reftable_writer *w, uint8_t *data, size_t len, · line 66

Quoted 10 lines

reftable/writer.c +3 -1

 
 		n = w->write(w->write_arg, zeroed, w->pending_padding);
-		if (n < 0)
+		if (n < 0) {
+			reftable_free(zeroed);
 			return n;
+		}
 
 		w->pending_padding = 0;
 		reftable_free(zeroed);

Makes sense.

reftable/writer.c static int writer_index_hash(struct reftable_writer *w, struct reftable_buf *has · line 267

Quoted 12 lines

reftable/writer.c +3 -1

@@ -256,8 +258,10 @@ static int writer_index_hash(struct reftable_writer *w, struct reftable_buf *has
 
 		reftable_buf_reset(&key->hash);
 		err = reftable_buf_add(&key->hash, hash->buf, hash->len);
-		if (err < 0)
+		if (err < 0) {
+			reftable_free(key);
 			return err;
+		}
 		tree_insert(&w->obj_index_tree, key,
 			    &obj_index_tree_node_compare);
 	} else {

Makes sense, as well. We want to add the node to the object index tree, but if a step before fails then we naturally want to free the node.

Patrick

lidongyan

2025-05-12 10:54 UTC #3 lore raw

Re: [PATCH v2] reftable/writer: fix memory leak if write fails

On 12/5/2025 at 16:25，Patrick Steinhardt <[email protected]> wrote：

Quoted 2 lines

Nit: I think it would be sensible to split these up into two commits, as they touch different areas of the code.

I will split them in the next patch.

Lidong Yan via GitGitGadget

author

2025-05-12 12:49 UTC #4 lore raw

v3 cover reftable/writer: fix memory leak when write fails

In reply to Lidong Yan via GitGitGadget

Lidong Yan (2):
reftable/writer: fix memory leak when padded_write() fails reftable/writer: fix memory leak when writer_index_hash() fails

 reftable/writer.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

base-commit: 6f84262c44a89851c3ae5a6e4c1a9d06b2068d75
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1958%2Fbrandb97%2Ffix-reftable-padded-write-leak-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1958/brandb97/fix-reftable-padded-write-leak-v3
Pull-Request: https://github.com/git/git/pull/1958

Range-diff vs v2:

1: 9329ae2d478 ! 1: 13ebdd672ff reftable/writer: fix memory leak if write fails
@@ Metadata
Author: Lidong Yan <[email protected]>

## Commit message ##
- reftable/writer: fix memory leak if write fails
+ reftable/writer: fix memory leak when padded_write() fails

In reftable/writer.c:padded_write(), if w->writer failed, zeroed allocated in reftable_calloc will leak. w->writer could be
@@ Commit message
some write error. Simply add reftable_free(zeroed) will solve this problem.

- In reftable/writer.c:writer_index_hash(), if reftable_buf_add failed,
- key allocated by reftable_malloc will not be insert into obj_index_tree
- thus leaks. Simple add reftable_free(key) will solve this problem.
-

Signed-off-by Lidong Yan <[email protected]>

## reftable/writer.c ##
@@ reftable/writer.c: static int padded_write(struct reftable_writer *w, uint8_t *d

w->pending_padding = 0;
reftable_free(zeroed);
-@@ reftable/writer.c: static int writer_index_hash(struct reftable_writer *w, struct reftable_buf *has -
- reftable_buf_reset(&key->hash);
- err = reftable_buf_add(&key->hash, hash->buf, hash->len);
-- if (err < 0)
-+ if (err < 0) {
-+ reftable_free(key);
- return err;
-+ }
- tree_insert(&w->obj_index_tree, key,
- &obj_index_tree_node_compare);
- } else {
-: ----------- > 2: 64f778ce2ba reftable/writer: fix memory leak when writer_index_hash() fails

Signature · 1 line

gitgitgadget

Lidong Yan via GitGitGadget

author

2025-05-12 12:49 UTC #5 lore raw

PATCH v3 1/2 reftable/writer: fix memory leak when `padded_write()` fails

From: Lidong Yan <[email protected]>

In reftable/writer.c:padded_write(), if w->writer failed, zeroed allocated in reftable_calloc will leak. w->writer could be reftable_write_data in reftable/stack.c, and could fail due to some write error. Simply add reftable_free(zeroed) will solve this problem.

Signed-off-by Lidong Yan <[email protected]>

---
 reftable/writer.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

reftable/writer.c +3 -1

index cb16f71be49e..3ceb37428887 100644
@@ -57,8 +57,10 @@ static int padded_write(struct reftable_writer *w, uint8_t *data, size_t len,
 			return -1;
 
 		n = w->write(w->write_arg, zeroed, w->pending_padding);
-		if (n < 0)
+		if (n < 0) {
+			reftable_free(zeroed);
 			return n;
+		}
 
 		w->pending_padding = 0;
 		reftable_free(zeroed);

Signature · 1 line

gitgitgadget

Lidong Yan via GitGitGadget

author

2025-05-12 12:49 UTC #6 lore raw

PATCH v3 2/2 reftable/writer: fix memory leak when `writer_index_hash()` fails

In reply to Lidong Yan via GitGitGadget

From: Lidong Yan <[email protected]>

In reftable/writer.c:writer_index_hash(), if reftable_buf_add failed, key allocated by reftable_malloc will not be insert into obj_index_tree thus leaks. Simple add reftable_free(key) will solve this problem.

Signed-off-by Lidong Yan <[email protected]>

---
 reftable/writer.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

reftable/writer.c +3 -1

index 3ceb37428887..3b4ebdd6dced 100644
@@ -258,8 +258,10 @@ static int writer_index_hash(struct reftable_writer *w, struct reftable_buf *has
 
 		reftable_buf_reset(&key->hash);
 		err = reftable_buf_add(&key->hash, hash->buf, hash->len);
-		if (err < 0)
+		if (err < 0) {
+			reftable_free(key);
 			return err;
+		}
 		tree_insert(&w->obj_index_tree, key,
 			    &obj_index_tree_node_compare);
 	} else {

Signature · 1 line

gitgitgadget

Patrick Steinhardt

2025-05-12 14:34 UTC #7 lore raw

Re: [PATCH v3 0/2] reftable/writer: fix memory leak when write fails

In reply to Lidong Yan via GitGitGadget

Quoted 6 lines · Lidong Yan via GitGitGadget

Lidong Yan (2):
reftable/writer: fix memory leak when padded_write() fails reftable/writer: fix memory leak when writer_index_hash() fails
 reftable/writer.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Thanks, these look good to me.

Patrick