Bug 206051

Summary: Rounding issue causes simplefb to reject EFI framebuffer data
Product: EFI Reporter: Christopher Head (bugs)
Component: VideoAssignee: EFI Virtual User (efi)
Status: NEW ---    
Severity: normal CC: jennifergodwin2099, nivedita
Priority: P1    
Hardware: All   
OS: Linux   
Kernel Version: 4.19.86 Subsystem:
Regression: No Bisected commit-id:

Description Christopher Head 2020-01-01 19:27:42 UTC
In drivers/firmware/efi/libstub/gop.c, the firmware provides the framebuffer’s pixels per scanline and height, setup_pixel_info converts pixels per scanline into lfb_linelength, and setup_gop{32,64} computes lfb_size as linelength*height.

In arch/x86/kernel/sysfb_simplefb.c, “length” is computed as height×stride rounded up to a multiple of the system page size. Then there is a check that the computed “length” is no greater than lfb_size.

In the case of an 800×600 32-bit-per-pixel framebuffer, lfb_width=800, lfb_height=600, lfb_linelength=800×4=3200, and lfb_size=3200×600=1,920,000. But then create_simplefb computes length=PAGE_ALIGN(600×3200)=1,921,024 and rejects it because length>lfb_size.

This doesn’t look like a firmware bug because the firmware isn’t specifying the byte size of the framebuffer at all, only the pixel format, width, and height. So all the firmware-provided values seem to be accurate, just the different layers of kernel code are disagreeing on whether lfb_size should be page-aligned. I don’t know whether it should be or not, and therefore which layer is wrong, either. But moving the “length = PAGE_ALIGN(length)” statement below the “length > size” check makes the simplefb in question work properly.

Note this was a Gentoo kernel tree, not a vanilla tree, but the two files involved look identical to vanilla git, so I think this is an upstream issue. There are three cleanup commits in gop.c from December 2019, but none of them would address this problem.
Comment 2 Arvind Sankar 2020-01-07 23:05:36 UTC
https://lore.kernel.org/lkml/20200107230410.2291947-1-nivedita@alum.mit.edu/

That sounds right.
Comment 3 Christopher Head 2020-01-12 05:00:19 UTC
That is the exact patch I used.