{"thread":{"id":"63432","subject":"[PATCH] reftable/writer: fix memory leak when `padded_write()` fails","startedAt":"2025-05-10T03:50:33Z","lastAt":"2025-05-12T14:34:05Z","messageCount":8,"participants":["Lidong Yan via GitGitGadget","Patrick Steinhardt","lidongyan"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"517720","messageId":"pull.1958.git.git.1746849030749.gitgitgadget@gmail.com","threadId":"63432","inReplyTo":null,"subject":"[PATCH] reftable/writer: fix memory leak when `padded_write()` fails","fromName":"Lidong Yan via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-05-10T03:50:30Z","receivedAt":"2025-05-10T03:50:33Z","isPatch":true,"sender":{"key":"yldhome2d2@gmail.com","avatar":"https://avatars.githubusercontent.com/u/77328395?v=4"},"body":"From: Lidong Yan <502024330056@smail.nju.edu.cn>\n\nIn reftable/writer.c:padded_write(), if w->writer failed, zeroed\nallocated in `reftable_calloc` will leak. w->writer could be\n`reftable_write_data` in reftable/stack.c, and could fail due to\nsome write error. Simply add reftable_free(zeroed) will solve this\nproblem.\n\nSigned-off-by: Lidong Yan <502024330056@smail.nju.edu.cn>\n---\n    reftable/writer: fix memory leak when padded_write() fails\n    \n    In reftable/writer.c:padded_write(), if w->writer failed, zeroed\n    allocated in reftable_calloc will leak. w->writer could be\n    reftable_write_data in reftable/stack.c, and could fail due to some\n    write error. Simply add reftable_free(zeroed) will solve this problem.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1958%2Fbrandb97%2Ffix-reftable-padded-write-leak-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1958/brandb97/fix-reftable-padded-write-leak-v1\nPull-Request: https://github.com/git/git/pull/1958\n\n reftable/writer.c | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/reftable/writer.c b/reftable/writer.c\nindex cb16f71be49..3ceb3742888 100644\n--- a/reftable/writer.c\n+++ b/reftable/writer.c\n@@ -57,8 +57,10 @@ static int padded_write(struct reftable_writer *w, uint8_t *data, size_t len,\n \t\t\treturn -1;\n \n \t\tn = w->write(w->write_arg, zeroed, w->pending_padding);\n-\t\tif (n < 0)\n+\t\tif (n < 0) {\n+\t\t\treftable_free(zeroed);\n \t\t\treturn n;\n+\t\t}\n \n \t\tw->pending_padding = 0;\n \t\treftable_free(zeroed);\n\nbase-commit: 6f84262c44a89851c3ae5a6e4c1a9d06b2068d75\n-- \ngitgitgadget\n"},{"id":"517787","messageId":"pull.1958.v2.git.git.1746980165245.gitgitgadget@gmail.com","threadId":"63432","inReplyTo":"pull.1958.git.git.1746849030749.gitgitgadget@gmail.com","subject":"[PATCH v2] reftable/writer: fix memory leak if write fails","fromName":"Lidong Yan via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-05-11T16:16:04Z","receivedAt":"2025-05-11T16:16:08Z","isPatch":true,"sender":{"key":"yldhome2d2@gmail.com","avatar":"https://avatars.githubusercontent.com/u/77328395?v=4"},"body":"From: Lidong Yan <502024330056@smail.nju.edu.cn>\n\nIn reftable/writer.c:padded_write(), if w->writer failed, zeroed\nallocated in `reftable_calloc` will leak. w->writer could be\n`reftable_write_data` in reftable/stack.c, and could fail due to\nsome write error. Simply add reftable_free(zeroed) will solve this\nproblem.\n\nIn reftable/writer.c:writer_index_hash(), if `reftable_buf_add` failed,\nkey allocated by `reftable_malloc` will not be insert into `obj_index_tree`\nthus leaks. Simple add reftable_free(key) will solve this problem.\n\nSigned-off-by: Lidong Yan <502024330056@smail.nju.edu.cn>\n---\n    reftable/writer: fix memory leak when padded_write() fails\n    \n    In reftable/writer.c:padded_write(), if w->writer failed, zeroed\n    allocated in reftable_calloc will leak. w->writer could be\n    reftable_write_data in reftable/stack.c, and could fail due to some\n    write error. Simply add reftable_free(zeroed) will solve this problem.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1958%2Fbrandb97%2Ffix-reftable-padded-write-leak-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1958/brandb97/fix-reftable-padded-write-leak-v2\nPull-Request: https://github.com/git/git/pull/1958\n\nRange-diff vs v1:\n\n 1:  2023d6791ef ! 1:  9329ae2d478 reftable/writer: fix memory leak when `padded_write()` fails\n     @@ Metadata\n      Author: Lidong Yan <502024330056@smail.nju.edu.cn>\n      \n       ## Commit message ##\n     -    reftable/writer: fix memory leak when `padded_write()` fails\n     +    reftable/writer: fix memory leak if write fails\n      \n          In reftable/writer.c:padded_write(), if w->writer failed, zeroed\n          allocated in `reftable_calloc` will leak. w->writer could be\n     @@ Commit message\n          some write error. Simply add reftable_free(zeroed) will solve this\n          problem.\n      \n     +    In reftable/writer.c:writer_index_hash(), if `reftable_buf_add` failed,\n     +    key allocated by `reftable_malloc` will not be insert into `obj_index_tree`\n     +    thus leaks. Simple add reftable_free(key) will solve this problem.\n     +\n          Signed-off-by: Lidong Yan <502024330056@smail.nju.edu.cn>\n      \n       ## reftable/writer.c ##\n     @@ reftable/writer.c: static int padded_write(struct reftable_writer *w, uint8_t *d\n       \n       \t\tw->pending_padding = 0;\n       \t\treftable_free(zeroed);\n     +@@ reftable/writer.c: static int writer_index_hash(struct reftable_writer *w, struct reftable_buf *has\n     + \n     + \t\treftable_buf_reset(&key->hash);\n     + \t\terr = reftable_buf_add(&key->hash, hash->buf, hash->len);\n     +-\t\tif (err < 0)\n     ++\t\tif (err < 0) {\n     ++\t\t\treftable_free(key);\n     + \t\t\treturn err;\n     ++\t\t}\n     + \t\ttree_insert(&w->obj_index_tree, key,\n     + \t\t\t    &obj_index_tree_node_compare);\n     + \t} else {\n\n\n reftable/writer.c | 8 ++++++--\n 1 file changed, 6 insertions(+), 2 deletions(-)\n\ndiff --git a/reftable/writer.c b/reftable/writer.c\nindex cb16f71be49..3b4ebdd6dce 100644\n--- a/reftable/writer.c\n+++ b/reftable/writer.c\n@@ -57,8 +57,10 @@ static int padded_write(struct reftable_writer *w, uint8_t *data, size_t len,\n \t\t\treturn -1;\n \n \t\tn = w->write(w->write_arg, zeroed, w->pending_padding);\n-\t\tif (n < 0)\n+\t\tif (n < 0) {\n+\t\t\treftable_free(zeroed);\n \t\t\treturn n;\n+\t\t}\n \n \t\tw->pending_padding = 0;\n \t\treftable_free(zeroed);\n@@ -256,8 +258,10 @@ static int writer_index_hash(struct reftable_writer *w, struct reftable_buf *has\n \n \t\treftable_buf_reset(&key->hash);\n \t\terr = reftable_buf_add(&key->hash, hash->buf, hash->len);\n-\t\tif (err < 0)\n+\t\tif (err < 0) {\n+\t\t\treftable_free(key);\n \t\t\treturn err;\n+\t\t}\n \t\ttree_insert(&w->obj_index_tree, key,\n \t\t\t    &obj_index_tree_node_compare);\n \t} else {\n\nbase-commit: 6f84262c44a89851c3ae5a6e4c1a9d06b2068d75\n-- \ngitgitgadget\n"},{"id":"517804","messageId":"aCGwgbJ_dbO4vKq3@pks.im","threadId":"63432","inReplyTo":"pull.1958.v2.git.git.1746980165245.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] reftable/writer: fix memory leak if write fails","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-05-12T08:25:37Z","receivedAt":"2025-05-12T08:25:46Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Sun, May 11, 2025 at 04:16:04PM +0000, Lidong Yan via GitGitGadget wrote:\n> From: Lidong Yan <502024330056@smail.nju.edu.cn>\n> \n> In reftable/writer.c:padded_write(), if w->writer failed, zeroed\n> allocated in `reftable_calloc` will leak. w->writer could be\n> `reftable_write_data` in reftable/stack.c, and could fail due to\n> some write error. Simply add reftable_free(zeroed) will solve this\n> problem.\n> \n> In reftable/writer.c:writer_index_hash(), if `reftable_buf_add` failed,\n> key allocated by `reftable_malloc` will not be insert into `obj_index_tree`\n> thus leaks. Simple add reftable_free(key) will solve this problem.\n\nNit: I think it would be sensible to split these up into two commits, as\nthey touch different areas of the code.\n\n> diff --git a/reftable/writer.c b/reftable/writer.c\n> index cb16f71be49..3b4ebdd6dce 100644\n> --- a/reftable/writer.c\n> +++ b/reftable/writer.c\n> @@ -57,8 +57,10 @@ static int padded_write(struct reftable_writer *w, uint8_t *data, size_t len,\n>  \t\t\treturn -1;\n>  \n>  \t\tn = w->write(w->write_arg, zeroed, w->pending_padding);\n> -\t\tif (n < 0)\n> +\t\tif (n < 0) {\n> +\t\t\treftable_free(zeroed);\n>  \t\t\treturn n;\n> +\t\t}\n>  \n>  \t\tw->pending_padding = 0;\n>  \t\treftable_free(zeroed);\n\nMakes sense.\n\n> @@ -256,8 +258,10 @@ static int writer_index_hash(struct reftable_writer *w, struct reftable_buf *has\n>  \n>  \t\treftable_buf_reset(&key->hash);\n>  \t\terr = reftable_buf_add(&key->hash, hash->buf, hash->len);\n> -\t\tif (err < 0)\n> +\t\tif (err < 0) {\n> +\t\t\treftable_free(key);\n>  \t\t\treturn err;\n> +\t\t}\n>  \t\ttree_insert(&w->obj_index_tree, key,\n>  \t\t\t    &obj_index_tree_node_compare);\n>  \t} else {\n\nMakes sense, as well. We want to add the node to the object index tree,\nbut if a step before fails then we naturally want to free the node.\n\nPatrick\n"},{"id":"517828","messageId":"4A6F44C3-D501-4CBC-9563-97714708C5A0@smail.nju.edu.cn","threadId":"63432","inReplyTo":"aCGwgbJ_dbO4vKq3@pks.im","subject":"Re: [PATCH v2] reftable/writer: fix memory leak if write fails","fromName":"lidongyan","fromEmail":"502024330056@smail.nju.edu.cn","sentAt":"2025-05-12T10:54:03Z","receivedAt":"2025-05-12T10:54:29Z","isPatch":true,"sender":{"key":"502024330056@smail.nju.edu.cn","avatar":"https://avatars.githubusercontent.com/u/77328395?v=4"},"body":"On 12/5/2025 at 16:25，Patrick Steinhardt <ps@pks.im> wrote：\n> Nit: I think it would be sensible to split these up into two commits, as\n> they touch different areas of the code.\nI will split them in the next patch.\n\n"},{"id":"517833","messageId":"pull.1958.v3.git.git.1747054145.gitgitgadget@gmail.com","threadId":"63432","inReplyTo":"pull.1958.v2.git.git.1746980165245.gitgitgadget@gmail.com","subject":"[PATCH v3 0/2] reftable/writer: fix memory leak when write fails","fromName":"Lidong Yan via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-05-12T12:49:02Z","receivedAt":"2025-05-12T12:49:08Z","isPatch":true,"sender":{"key":"yldhome2d2@gmail.com","avatar":"https://avatars.githubusercontent.com/u/77328395?v=4"},"body":"Lidong Yan (2):\n  reftable/writer: fix memory leak when `padded_write()` fails\n  reftable/writer: fix memory leak when `writer_index_hash()` fails\n\n reftable/writer.c | 8 ++++++--\n 1 file changed, 6 insertions(+), 2 deletions(-)\n\n\nbase-commit: 6f84262c44a89851c3ae5a6e4c1a9d06b2068d75\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1958%2Fbrandb97%2Ffix-reftable-padded-write-leak-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1958/brandb97/fix-reftable-padded-write-leak-v3\nPull-Request: https://github.com/git/git/pull/1958\n\nRange-diff vs v2:\n\n 1:  9329ae2d478 ! 1:  13ebdd672ff reftable/writer: fix memory leak if write fails\n     @@ Metadata\n      Author: Lidong Yan <502024330056@smail.nju.edu.cn>\n      \n       ## Commit message ##\n     -    reftable/writer: fix memory leak if write fails\n     +    reftable/writer: fix memory leak when `padded_write()` fails\n      \n          In reftable/writer.c:padded_write(), if w->writer failed, zeroed\n          allocated in `reftable_calloc` will leak. w->writer could be\n     @@ Commit message\n          some write error. Simply add reftable_free(zeroed) will solve this\n          problem.\n      \n     -    In reftable/writer.c:writer_index_hash(), if `reftable_buf_add` failed,\n     -    key allocated by `reftable_malloc` will not be insert into `obj_index_tree`\n     -    thus leaks. Simple add reftable_free(key) will solve this problem.\n     -\n          Signed-off-by: Lidong Yan <502024330056@smail.nju.edu.cn>\n      \n       ## reftable/writer.c ##\n     @@ reftable/writer.c: static int padded_write(struct reftable_writer *w, uint8_t *d\n       \n       \t\tw->pending_padding = 0;\n       \t\treftable_free(zeroed);\n     -@@ reftable/writer.c: static int writer_index_hash(struct reftable_writer *w, struct reftable_buf *has\n     - \n     - \t\treftable_buf_reset(&key->hash);\n     - \t\terr = reftable_buf_add(&key->hash, hash->buf, hash->len);\n     --\t\tif (err < 0)\n     -+\t\tif (err < 0) {\n     -+\t\t\treftable_free(key);\n     - \t\t\treturn err;\n     -+\t\t}\n     - \t\ttree_insert(&w->obj_index_tree, key,\n     - \t\t\t    &obj_index_tree_node_compare);\n     - \t} else {\n -:  ----------- > 2:  64f778ce2ba reftable/writer: fix memory leak when `writer_index_hash()` fails\n\n-- \ngitgitgadget\n"},{"id":"517834","messageId":"13ebdd672ff1a76ef684d9260b782c750c65ddba.1747054145.git.gitgitgadget@gmail.com","threadId":"63432","inReplyTo":"pull.1958.v3.git.git.1747054145.gitgitgadget@gmail.com","subject":"[PATCH v3 1/2] reftable/writer: fix memory leak when `padded_write()` fails","fromName":"Lidong Yan via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-05-12T12:49:03Z","receivedAt":"2025-05-12T12:49:09Z","isPatch":true,"sender":{"key":"yldhome2d2@gmail.com","avatar":"https://avatars.githubusercontent.com/u/77328395?v=4"},"body":"From: Lidong Yan <502024330056@smail.nju.edu.cn>\n\nIn reftable/writer.c:padded_write(), if w->writer failed, zeroed\nallocated in `reftable_calloc` will leak. w->writer could be\n`reftable_write_data` in reftable/stack.c, and could fail due to\nsome write error. Simply add reftable_free(zeroed) will solve this\nproblem.\n\nSigned-off-by: Lidong Yan <502024330056@smail.nju.edu.cn>\n---\n reftable/writer.c | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/reftable/writer.c b/reftable/writer.c\nindex cb16f71be49e..3ceb37428887 100644\n--- a/reftable/writer.c\n+++ b/reftable/writer.c\n@@ -57,8 +57,10 @@ static int padded_write(struct reftable_writer *w, uint8_t *data, size_t len,\n \t\t\treturn -1;\n \n \t\tn = w->write(w->write_arg, zeroed, w->pending_padding);\n-\t\tif (n < 0)\n+\t\tif (n < 0) {\n+\t\t\treftable_free(zeroed);\n \t\t\treturn n;\n+\t\t}\n \n \t\tw->pending_padding = 0;\n \t\treftable_free(zeroed);\n-- \ngitgitgadget\n\n"},{"id":"517835","messageId":"64f778ce2ba6a3010d284daeb09ed2d591b54565.1747054145.git.gitgitgadget@gmail.com","threadId":"63432","inReplyTo":"pull.1958.v3.git.git.1747054145.gitgitgadget@gmail.com","subject":"[PATCH v3 2/2] reftable/writer: fix memory leak when `writer_index_hash()` fails","fromName":"Lidong Yan via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-05-12T12:49:04Z","receivedAt":"2025-05-12T12:49:10Z","isPatch":true,"sender":{"key":"yldhome2d2@gmail.com","avatar":"https://avatars.githubusercontent.com/u/77328395?v=4"},"body":"From: Lidong Yan <502024330056@smail.nju.edu.cn>\n\nIn reftable/writer.c:writer_index_hash(), if `reftable_buf_add` failed,\nkey allocated by `reftable_malloc` will not be insert into `obj_index_tree`\nthus leaks. Simple add reftable_free(key) will solve this problem.\n\nSigned-off-by: Lidong Yan <502024330056@smail.nju.edu.cn>\n---\n reftable/writer.c | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/reftable/writer.c b/reftable/writer.c\nindex 3ceb37428887..3b4ebdd6dced 100644\n--- a/reftable/writer.c\n+++ b/reftable/writer.c\n@@ -258,8 +258,10 @@ static int writer_index_hash(struct reftable_writer *w, struct reftable_buf *has\n \n \t\treftable_buf_reset(&key->hash);\n \t\terr = reftable_buf_add(&key->hash, hash->buf, hash->len);\n-\t\tif (err < 0)\n+\t\tif (err < 0) {\n+\t\t\treftable_free(key);\n \t\t\treturn err;\n+\t\t}\n \t\ttree_insert(&w->obj_index_tree, key,\n \t\t\t    &obj_index_tree_node_compare);\n \t} else {\n-- \ngitgitgadget\n"},{"id":"517851","messageId":"aCIG2W60WbZtW8Lu@pks.im","threadId":"63432","inReplyTo":"pull.1958.v3.git.git.1747054145.gitgitgadget@gmail.com","subject":"Re: [PATCH v3 0/2] reftable/writer: fix memory leak when write fails","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-05-12T14:34:01Z","receivedAt":"2025-05-12T14:34:05Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, May 12, 2025 at 12:49:02PM +0000, Lidong Yan via GitGitGadget wrote:\n> Lidong Yan (2):\n>   reftable/writer: fix memory leak when `padded_write()` fails\n>   reftable/writer: fix memory leak when `writer_index_hash()` fails\n> \n>  reftable/writer.c | 8 ++++++--\n>  1 file changed, 6 insertions(+), 2 deletions(-)\n\nThanks, these look good to me.\n\nPatrick\n"}]}