| Message ID | 33b93fdf-bf16-49b8-aec2-0b2c19f5c471@suse.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | gunzip: don't leak memory on error paths | expand |
On 06/05/2024 9:08 am, Jan Beulich wrote: > While decompression errors are likely going to be fatal to Xen's boot > process anyway, the latest with the goal of doing multiple decompressor > runs it is likely better to avoid leaks even on error paths. All the > more when this way code size actually shrinks a tiny bit. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> Right now, all errors in gzip are panic()'s, although that is something that will need addressing in due course. Given that I'm shuffling Daniel's series a little in this code area anyway, I'll slot this patch in ahead which I think will be the least disruptive overall. ~Andrew
--- a/xen/common/gzip/inflate.c +++ b/xen/common/gzip/inflate.c @@ -757,16 +757,14 @@ static int noinline __init inflate_fixed } /* decompress until an end-of-block code */ - if (inflate_codes(tl, td, bl, bd)) { - free(l); - return 1; - } + i = inflate_codes(tl, td, bl, bd); /* free the decoding tables, return */ free(l); huft_free(tl); huft_free(td); - return 0; + + return !!i; } /* @@ -940,19 +938,17 @@ static int noinline __init inflate_dynam DEBG("dyn6 "); /* decompress until an end-of-block code */ - if (inflate_codes(tl, td, bl, bd)) { - ret = 1; - goto out; - } + ret = !!inflate_codes(tl, td, bl, bd); - DEBG("dyn7 "); + if (!ret) + DEBG("dyn7 "); /* free the decoding tables, return */ huft_free(tl); huft_free(td); - DEBG(">"); - ret = 0; + if (!ret) + DEBG(">"); out: free(ll); return ret;
While decompression errors are likely going to be fatal to Xen's boot process anyway, the latest with the goal of doing multiple decompressor runs it is likely better to avoid leaks even on error paths. All the more when this way code size actually shrinks a tiny bit. Signed-off-by: Jan Beulich <jbeulich@suse.com> --- This is quite the opposite of Coverity reporting use-after-free-s and free-after-free-s in inflate_dynamic() for tl and td, for an unclear to me reason.