git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Patrick Steinhardt <ps@pks.im>
Date
May 12, 2025, 08:25 UTC
Message-ID
<aCGwgbJ_dbO4vKq3@pks.im>
In-Reply-To
<pull.1958.v2.git.git.1746980165245.gitgitgadget@gmail.com>
On Sun, May 11, 2025 at 04:16:04PM +0000, Lidong Yan via GitGitGadget wrote:
Show 11 quoted lines
> 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.
Show 16 quoted lines
> 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.
Show 12 quoted lines
> @@ -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
Previous: Lidong Yan via GitGitGadgetNext: lidongyan
Message 3 of 8 in “reftable/writer: fix memory leak when `padded_write()` fails”
  1. reftable/writer: fix memory leak when `padded_write()` failsLidong Yan via GitGitGadget, May 10, 2025
  2. reftable/writer: fix memory leak if write failsLidong Yan via GitGitGadget, May 11, 2025
  3. Patrick SteinhardtMay 12, 2025
  4. lidongyanMay 12, 2025
  5. 0/2 reftable/writer: fix memory leak when write failsLidong Yan via GitGitGadget, May 12, 2025
  6. 1/2 reftable/writer: fix memory leak when `padded_write()` failsLidong Yan via GitGitGadget, May 12, 2025
  7. 2/2 reftable/writer: fix memory leak when `writer_index_hash()` failsLidong Yan via GitGitGadget, May 12, 2025
  8. Patrick SteinhardtMay 12, 2025

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.