# [PATCH] reftable/writer: fix memory leak when `padded_write()` fails 8 messages from 2025-05-10 to 2025-05-12. Participants: Lidong Yan via GitGitGadget, Patrick Steinhardt, lidongyan. Thread: https://gitlist.dev/t/63432 ## Lidong Yan via GitGitGadget, 2025-05-10 03:50 Subject: [PATCH] reftable/writer: fix memory leak when `padded_write()` fails Message-ID: URL: https://gitlist.dev/e/pull.1958.git.git.1746849030749.gitgitgadget%40gmail.com ``` From: Lidong Yan <502024330056@smail.nju.edu.cn> 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 <502024330056@smail.nju.edu.cn> --- 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(-) diff --git a/reftable/writer.c b/reftable/writer.c index cb16f71be49..3ceb3742888 100644 --- a/reftable/writer.c +++ b/reftable/writer.c @@ -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 -- gitgitgadget ``` ## Lidong Yan via GitGitGadget, 2025-05-11 16:16 Subject: [PATCH v2] reftable/writer: fix memory leak if write fails Message-ID: URL: https://gitlist.dev/e/pull.1958.v2.git.git.1746980165245.gitgitgadget%40gmail.com In-Reply-To: ``` From: Lidong Yan <502024330056@smail.nju.edu.cn> 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 <502024330056@smail.nju.edu.cn> --- 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 <502024330056@smail.nju.edu.cn> ## 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 <502024330056@smail.nju.edu.cn> ## 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(-) diff --git a/reftable/writer.c b/reftable/writer.c index cb16f71be49..3b4ebdd6dce 100644 --- a/reftable/writer.c +++ b/reftable/writer.c @@ -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 -- gitgitgadget ``` ## Patrick Steinhardt, 2025-05-12 08:25 Subject: Re: [PATCH v2] reftable/writer: fix memory leak if write fails Message-ID: URL: https://gitlist.dev/e/aCGwgbJ_dbO4vKq3%40pks.im In-Reply-To: ``` On Sun, May 11, 2025 at 04:16:04PM +0000, Lidong Yan via GitGitGadget wrote: > From: Lidong Yan <502024330056@smail.nju.edu.cn> > > 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. > diff --git a/reftable/writer.c b/reftable/writer.c > index cb16f71be49..3b4ebdd6dce 100644 > --- a/reftable/writer.c > +++ b/reftable/writer.c > @@ -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); Makes sense. > @@ -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 Subject: Re: [PATCH v2] reftable/writer: fix memory leak if write fails Message-ID: <4A6F44C3-D501-4CBC-9563-97714708C5A0@smail.nju.edu.cn> URL: https://gitlist.dev/e/4A6F44C3-D501-4CBC-9563-97714708C5A0%40smail.nju.edu.cn In-Reply-To: ``` On 12/5/2025 at 16:25，Patrick Steinhardt wrote： > 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, 2025-05-12 12:49 Subject: [PATCH v3 0/2] reftable/writer: fix memory leak when write fails Message-ID: URL: https://gitlist.dev/e/pull.1958.v3.git.git.1747054145.gitgitgadget%40gmail.com In-Reply-To: ``` 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 <502024330056@smail.nju.edu.cn> ## 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 <502024330056@smail.nju.edu.cn> ## 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 -- gitgitgadget ``` ## Lidong Yan via GitGitGadget, 2025-05-12 12:49 Subject: [PATCH v3 1/2] reftable/writer: fix memory leak when `padded_write()` fails Message-ID: <13ebdd672ff1a76ef684d9260b782c750c65ddba.1747054145.git.gitgitgadget@gmail.com> URL: https://gitlist.dev/e/13ebdd672ff1a76ef684d9260b782c750c65ddba.1747054145.git.gitgitgadget%40gmail.com In-Reply-To: ``` From: Lidong Yan <502024330056@smail.nju.edu.cn> 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 <502024330056@smail.nju.edu.cn> --- reftable/writer.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/reftable/writer.c b/reftable/writer.c index cb16f71be49e..3ceb37428887 100644 --- a/reftable/writer.c +++ b/reftable/writer.c @@ -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); -- gitgitgadget ``` ## Lidong Yan via GitGitGadget, 2025-05-12 12:49 Subject: [PATCH v3 2/2] reftable/writer: fix memory leak when `writer_index_hash()` fails Message-ID: <64f778ce2ba6a3010d284daeb09ed2d591b54565.1747054145.git.gitgitgadget@gmail.com> URL: https://gitlist.dev/e/64f778ce2ba6a3010d284daeb09ed2d591b54565.1747054145.git.gitgitgadget%40gmail.com In-Reply-To: ``` From: Lidong Yan <502024330056@smail.nju.edu.cn> 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 <502024330056@smail.nju.edu.cn> --- reftable/writer.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/reftable/writer.c b/reftable/writer.c index 3ceb37428887..3b4ebdd6dced 100644 --- a/reftable/writer.c +++ b/reftable/writer.c @@ -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 { -- gitgitgadget ``` ## Patrick Steinhardt, 2025-05-12 14:34 Subject: Re: [PATCH v3 0/2] reftable/writer: fix memory leak when write fails Message-ID: URL: https://gitlist.dev/e/aCIG2W60WbZtW8Lu%40pks.im In-Reply-To: ``` On Mon, May 12, 2025 at 12:49:02PM +0000, Lidong Yan via GitGitGadget wrote: > 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 ```