[PATCH v2] x86/bzimage: Check that the payload lies within the image

[Stefano Stabellini <stefano.stabellini@xxxxxxx>]

bzimage_check() doesn't validate setup_sects, payload_offset or
payload_length against the image size.  A truncated or malformed bzImage
therefore makes bzimage_headroom() and bzimage_parse() read beyond the
end of the boot module, which shows up as a fatal page fault, a bogus
headroom, or a decompression failure, rather than as a bad image.

Reject such a header up front.  bzimage_parse() passes bzimage_check() a
length which includes the decompression headroom in front of the image,
so subtract it to check against the image alone.

Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxx>
---
Changes in v2:
- drop the UL suffix: setup_sects is 8 bits wide, so the multiplication
  cannot overflow, and the offset is now computed the same way as where
  bzimage_headroom() and bzimage_parse() use it
- reword the error message
- drop the dereferences from the comment

 xen/arch/x86/bzimage.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/bzimage.c b/xen/arch/x86/bzimage.c
index 66f648f311..41f19221ac 100644
--- a/xen/arch/x86/bzimage.c
+++ b/xen/arch/x86/bzimage.c
@@ -54,6 +54,8 @@ struct __packed setup_header {
 
 static __init int bzimage_check(struct setup_header *hdr, unsigned long len)
 {
+    unsigned long payload_start;
+
     if ( len < sizeof(struct setup_header) )
         return 0;
 
@@ -65,6 +67,15 @@ static __init int bzimage_check(struct setup_header *hdr, 
unsigned long len)
            hdr->version >> 8, hdr->version & 0xff);
         return -EINVAL;
     }
+
+    payload_start = (hdr->setup_sects + 1) * 512 + hdr->payload_offset;
+    if ( payload_start > len || hdr->payload_length > len - payload_start )
+    {
+        printk("bzImage payload %#x@%#lx past the image size (%#lx)\n",
+               hdr->payload_length, payload_start, len);
+        return -EINVAL;
+    }
+
     return 1;
 }
 
@@ -107,7 +118,8 @@ int __init bzimage_parse(void *image_base, void 
**image_start,
                          unsigned long *image_len)
 {
     struct setup_header *hdr = (struct setup_header *)(*image_start);
-    int err = bzimage_check(hdr, *image_len);
+    /* image_len also counts the headroom ahead of image_start. */
+    int err = bzimage_check(hdr, *image_len - (*image_start - image_base));
     unsigned long output_len;
 
     if ( err < 0 )
-- 
2.47.3