Re: [Patch] Fix VESA VBE BIOS support in GRUB Legacy

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 11/21/2010 12:29 AM, Gregg Levine wrote:
> On Sat, Nov 20, 2010 at 1:54 PM, Martin Decky <address@hidden> wrote:
>   
>> Hello folks,
>>
>> GRUB Legacy has two builtin commands "vbeprobe" and "testvbe" which are
>> intended to test VESA VBE BIOS graphics modes (e.g. for manually testing the
>> correct Linux VESA framebuffer mode before setting the vga= Linux kernel
>> command line value).
>>
>> However, on many recent graphics cards which are VESA 3.0 compliant these
>> commands do not work. The reason is a wrong definition of the vbe_mode
>> structure in stage2/shared.h. The VBE mode info structure should be 256
>> bytes long, but here it is actually only 255 bytes long, the array "unsigned
>> char reserved3[189]" should be actually declared as "unsigned char
>> reserved3[190]" (frankly, the same mistake is also present in the official
>> VESA VBE 3.0 specification document).
>>
>> As the vbe_mode structure is allocated on stack in vbeprobe_func() and
>> testvbe_func() (stage2/builtins.c), if the VESA BIOS actually stores all 256
>> bytes in the 0x4f01 call (Get Mode Info), the last byte overruns the
>> structure and ends in a different variable on the stack (mode_number in case
>> of vbeprobe_func()), thus rendering both routines non-functional.
>>
>>
>> Please see a trivial patch attached to this email which solves the issue.
>> Thanks for committing it to the mainline of GRUB Legacy -- I understand that
>> GRUB Legacy is no longer in development, but this is a stack overrun type of
>> bug which should be fixed. I have not checked GRUB 2 whether it contains the
>> same mistake, but somebody might have a look on it.
>>
>>
>> Best regards
>>
>> Martin Decky
>> --
>>    ------------------------------------------------------------------
>>    Mgr. Martin Decky
>>     Department of Distributed and Dependable Systems,
>>     Faculty of Mathematics and Physics,
>>     Charles University in Prague,
>>     Czech Republic
>>
>>    address@hidden                              address@hidden
>>    http://d3s.mff.cuni.cz/~decky/                 http://www.decky.cz
>>    ------------------------------------------------------------------
>>     
> Hello!
> We aren't committing that patch against the source code. You are aware
> that Grub-Legacy is just that. It is legacy because support, if it can
> be called that, stopped when GRUB2 started to be developed. Now if you
> want to add it to your tree and maintain it, it too would be
> acceptable. But the final comment will come from the individual who
> normally responds to these sort of statements.
>   
I can only repeat your words and point out that in GRUB2 we already have:
  /* Reserved field to make structure to be 256 bytes long, VESA BIOS
     Extension 3.0 Specification says to reserve 189 bytes here but
     that doesn't make structure to be 256 bytes.  So additional one is
     added here.  */
  grub_uint8_t reserved4[189 + 1];
} __attribute__ ((packed));
Actually even if we commit anything against GRUB Legacy, it's unlikely
to be propagated by distros at this point anyway because I doubt anyone
looks at the repo where there haven't been any commits for significant
time. We keep that repo only for reference reasons but it isn't and
shouldn't be considered as anything functional by now.

> -----
> Gregg C Levine address@hidden
> "This signature fought the Time Wars, time and again."
>
> _______________________________________________
> Bug-grub mailing list
> address@hidden
> http://lists.gnu.org/mailman/listinfo/bug-grub
>
>   


-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko

signature.asc
Description: OpenPGP digital signature