inflate_codes() copies each length/distance pair from that many bytes
behind the write position, and does not check the distance against the
bytes written so far. RFC 1951 (section 3.2.3) forbids a distance
reaching back before the start of the output, but a corrupt or crafted
image can carry one. The source position is then taken modulo WSIZE,
and because Xen's window is the output buffer itself, sized for the
image and not 2 GiB long, the copy reads from up to 32 KiB below offset
2 GiB of that buffer: memory the decompressor was never given, whose
contents end up in the unpacked kernel. The CRC32 and length checks of
the trailer only reject the image afterwards, once that memory has been
read.

Check the distance as soon as it is decoded, against the bytes written
so far, the current window position plus what earlier flushes of the
window wrote out, and refuse the block as invalid, as for an undefined
code, when it reaches back further. Xen then stops with "invalid
compressed format (err=1)", as for any other invalid block, without
reading memory outside the output.

Fixes: e2b1ebf4de50 ("x86: Support booting a bzImage format domain 0 kernel.")
Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Anthony PERARD <anthony.perard@vates.tech>
Cc: Michal Orzel <michal.orzel@amd.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Julien Grall <julien@xen.org>
Cc: Roger Pau Monné <roger@xenproject.org>
---
 xen/common/gzip/inflate.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/xen/common/gzip/inflate.c b/xen/common/gzip/inflate.c
index cb146555c8..a40e83b250 100644
--- a/xen/common/gzip/inflate.c
+++ b/xen/common/gzip/inflate.c
@@ -603,8 +603,13 @@ static int __init inflate_codes(
                 } while ((e = (t = t->v.t + ((unsigned)b & mask_bits[e]))->e) > 16);
             DUMPBITS(t->b);
             NEEDBITS(s, e);
-            d = w - t->v.n - ((unsigned)b & mask_bits[e]);
+            d = t->v.n + ((unsigned)b & mask_bits[e]);
             DUMPBITS(e);
+
+            if (d > w + s->bytes_out)
+                return 1;
+
+            d = w - d;
             Tracevv((stderr,"\\[%d,%d]", w-d, n));
 
             /* do the copy */
-- 
2.47.3

On 30.10.2026 21:15, Stefano Stabellini wrote:
> --- a/xen/common/gzip/inflate.c
> +++ b/xen/common/gzip/inflate.c
> @@ -603,8 +603,13 @@ static int __init inflate_codes(
>                  } while ((e = (t = t->v.t + ((unsigned)b & mask_bits[e]))->e) > 16);
>              DUMPBITS(t->b);
>              NEEDBITS(s, e);
> -            d = w - t->v.n - ((unsigned)b & mask_bits[e]);
> +            d = t->v.n + ((unsigned)b & mask_bits[e]);
>              DUMPBITS(e);
> +
> +            if (d > w + s->bytes_out)
> +                return 1;
> +
> +            d = w - d;

I've been staring at this (and the surrounding) code for far too long, yet I
still can't see how this works. w being the window position (which can be as
low as 0), this subtraction is almost certain to underflow in many cases (not
just in the updated code, but also in the original one). Yet later not only w
but also d are used to directly index s->window[]. If the subtraction
underflowed, how would such indexing be valid at all? I must be overlooking
something.

>              Tracevv((stderr,"\\[%d,%d]", w-d, n));

Imo this would now better become

            Tracevv((stderr,"\\[%d,%d]", d, n));
            d = w - d;

Jan