Re: [PATCH] reftable: fix unlikely leak on API error
From: Patrick Steinhardt <hidden>
Date: 2026-06-29 06:22:03
On Sun, Jun 28, 2026 at 05:03:14AM -0400, Jeff King wrote:
If the reftable writer sees a bogus block size, we return with REFTABLE_API_ERROR, leaking the reftable_writer struct we previously allocated. Originally this case was a BUG(), but it became a regular return in 445f9f4f35 (reftable: stop using `BUG()` in trivial cases, 2025-02-18). We could obviously fix it by calling "reftable_free(wp)". But we can observe that we never use the allocated "wp" until after we've validated the input options. So let's just bump the allocation down. That fixes the leak, and I think makes the flow of the function more logical (we validate our inputs before doing any work).
Another alternative would be to create a common exit path where we free the structure when we're about to return an error. But that might not even be worth it.
quoted hunk ↗ jump to hunk
diff --git a/reftable/writer.c b/reftable/writer.c index 0133b64975..1bd4aa388b 100644 --- a/reftable/writer.c +++ b/reftable/writer.c@@ -152,16 +152,16 @@ int reftable_writer_new(struct reftable_writer **out, struct reftable_write_options opts = {0}; struct reftable_writer *wp; - wp = reftable_calloc(1, sizeof(*wp)); - if (!wp) - return REFTABLE_OUT_OF_MEMORY_ERROR; - if (_opts) opts = *_opts; options_set_defaults(&opts); if (opts.block_size >= (1 << 24)) return REFTABLE_API_ERROR; + wp = reftable_calloc(1, sizeof(*wp)); + if (!wp) + return REFTABLE_OUT_OF_MEMORY_ERROR; + reftable_buf_init(&wp->block_writer_data.last_key); reftable_buf_init(&wp->last_key); reftable_buf_init(&wp->scratch);
Makes sense. There's another early return in this function, but there we already know to free the writer. Thanks! Patrick