Per the VBE 2.0 specification, the VBE controller information is 512
bytes long when the "VBE2" signature is provided, instead of the
original 256 bytes.

src/bootsplash.c uses the original pre-VBE-2.0 256-byte structure while
also filling in the "VBE2" signature, so a video BIOS that makes use of
the VBE2 OemData area could write past the end of the allocated region.

The original bootsplash code did not have this bug; it was introduced
when the bootsplash VBE structures were merged with the VGA ROM struct
definitions.

Fixes: 69e941c159ed ("Merge bootsplash and VGA ROM vbe structure definitions")
Signed-off-by: Daniel Verkamp <daniel@drv.nu>
---

v2 fixes the inverse bug introduced by the original patch - the vgabios
would memset too much data if the caller did not request VBE2 data.

 src/std/vbe.h | 2 ++
 vgasrc/vbe.c  | 4 +++-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/std/vbe.h b/src/std/vbe.h
index 94b4ad86..fe96f5ec 100644
--- a/src/std/vbe.h
+++ b/src/std/vbe.h
@@ -18,6 +18,8 @@ struct vbe_info {
     struct segoff_s oem_product_string;
     struct segoff_s oem_revision_string;
     u8 reserved[222];
+    /* VBE 2.0 */
+    u8 oem_data[256];
 } PACKED;
 
 struct vbe_mode_info {
diff --git a/vgasrc/vbe.c b/vgasrc/vbe.c
index 66afb011..91abc9ab 100644
--- a/vgasrc/vbe.c
+++ b/vgasrc/vbe.c
@@ -32,16 +32,18 @@ vbe_104f00(struct bregs *regs)
 {
     u16 seg = regs->es;
     struct vbe_info *info = (void*)(regs->di+0);
+    size_t info_size = offsetof(struct vbe_info, oem_data);
 
     if (GET_FARVAR(seg, info->signature) == VBE2_SIGNATURE) {
         dprintf(4, "Get VBE Controller: VBE2 Signature found\n");
+        info_size = sizeof(*info);
     } else if (GET_FARVAR(seg, info->signature) == VESA_SIGNATURE) {
         dprintf(4, "Get VBE Controller: VESA Signature found\n");
     } else {
         dprintf(4, "Get VBE Controller: Invalid Signature\n");
     }
 
-    memset_far(seg, info, 0, sizeof(*info));
+    memset_far(seg, info, 0, info_size);
 
     SET_FARVAR(seg, info->signature, VESA_SIGNATURE);
 
-- 
2.43.0

_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org

On Thu, Mar 07, 2024 at 01:08:27AM -0800, Daniel Verkamp wrote:
> Per the VBE 2.0 specification, the VBE controller information is 512
> bytes long when the "VBE2" signature is provided, instead of the
> original 256 bytes.
> 
> src/bootsplash.c uses the original pre-VBE-2.0 256-byte structure while
> also filling in the "VBE2" signature, so a video BIOS that makes use of
> the VBE2 OemData area could write past the end of the allocated region.
> 
> The original bootsplash code did not have this bug; it was introduced
> when the bootsplash VBE structures were merged with the VGA ROM struct
> definitions.
> 
> Fixes: 69e941c159ed ("Merge bootsplash and VGA ROM vbe structure definitions")
> Signed-off-by: Daniel Verkamp <daniel@drv.nu>

Nice find!  I committed this change.

Thanks,
-Kevin


> ---
> 
> v2 fixes the inverse bug introduced by the original patch - the vgabios
> would memset too much data if the caller did not request VBE2 data.
> 
>  src/std/vbe.h | 2 ++
>  vgasrc/vbe.c  | 4 +++-
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/src/std/vbe.h b/src/std/vbe.h
> index 94b4ad86..fe96f5ec 100644
> --- a/src/std/vbe.h
> +++ b/src/std/vbe.h
> @@ -18,6 +18,8 @@ struct vbe_info {
>      struct segoff_s oem_product_string;
>      struct segoff_s oem_revision_string;
>      u8 reserved[222];
> +    /* VBE 2.0 */
> +    u8 oem_data[256];
>  } PACKED;
>  
>  struct vbe_mode_info {
> diff --git a/vgasrc/vbe.c b/vgasrc/vbe.c
> index 66afb011..91abc9ab 100644
> --- a/vgasrc/vbe.c
> +++ b/vgasrc/vbe.c
> @@ -32,16 +32,18 @@ vbe_104f00(struct bregs *regs)
>  {
>      u16 seg = regs->es;
>      struct vbe_info *info = (void*)(regs->di+0);
> +    size_t info_size = offsetof(struct vbe_info, oem_data);
>  
>      if (GET_FARVAR(seg, info->signature) == VBE2_SIGNATURE) {
>          dprintf(4, "Get VBE Controller: VBE2 Signature found\n");
> +        info_size = sizeof(*info);
>      } else if (GET_FARVAR(seg, info->signature) == VESA_SIGNATURE) {
>          dprintf(4, "Get VBE Controller: VESA Signature found\n");
>      } else {
>          dprintf(4, "Get VBE Controller: Invalid Signature\n");
>      }
>  
> -    memset_far(seg, info, 0, sizeof(*info));
> +    memset_far(seg, info, 0, info_size);
>  
>      SET_FARVAR(seg, info->signature, VESA_SIGNATURE);
>  
> -- 
> 2.43.0
> 
> _______________________________________________
> SeaBIOS mailing list -- seabios@seabios.org
> To unsubscribe send an email to seabios-leave@seabios.org
_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org