[PATCH] x86/EFI: fix padding of EDD strings

[PATCH] x86/EFI: fix padding of EDD strings

[Anandu Padinhare]

edd_put_string() decrements n in its copy loop condition even when the
loop terminates because the end of the source string was reached.  The
padding loop then writes one space too few, leaving the final byte of
the field untouched (zero, as boot_edd_info[] lives in .bss).  On an EFI
boot this yields e.g. a host bus type of "PCI\0" and an interface type
of "ATAPI  \0", whereas BIOSes space-pad these fields (SeaBIOS reports
"PCI " and "ATA     ").

Furthermore, if the string exactly filled the field, n would wrap to
SIZE_MAX after the copy loop and the padding loop would write past the
end of the field.  No caller currently passes such a string.

Only decrement n when a character was actually copied.  While touching
the function, also correct its indentation.

Fixes: 51f03ec421fb ("x86-64/EFI: construct EDD data from device path protocol information")
Assisted-by: Claude
Signed-off-by: Anandu Padinhare <Anandu.Padinhare@electrifex.com>
---
Tested in QEMU (i440fx, OVMF, 2 IDE + 2 virtio-scsi disks) with a debug
build and a Linux 7.1 PV Dom0, booting xen.efi directly and via GRUB2
multiboot2.  /sys/firmware/edd/int13_dev8?/raw_data showed
host_bus_type "50 43 49 00" and interface types ending in 00 before
the patch, and "50 43 49 20" with fully space-padded interface types
after it, on all five disks.

 xen/arch/x86/efi/efi-boot.h | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
index d738b839ee..dab61c2d4b 100644
--- a/xen/arch/x86/efi/efi-boot.h
+++ b/xen/arch/x86/efi/efi-boot.h
@@ -30,13 +30,13 @@ static module_t __initdata mb_modules[5];
 
 static void __init edd_put_string(u8 *dst, size_t n, const char *src)
 {
-    while ( n-- && *src )
-       *dst++ = *src++;
+    for ( ; n && *src; --n )
+        *dst++ = *src++;
     if ( *src )
-       PrintErrMesg(L"Internal error populating EDD info",
-                    EFI_BUFFER_TOO_SMALL);
+        PrintErrMesg(L"Internal error populating EDD info",
+                     EFI_BUFFER_TOO_SMALL);
     while ( n-- )
-       *dst++ = ' ';
+        *dst++ = ' ';
 }
 #define edd_put_string(d, s) edd_put_string(d, ARRAY_SIZE(d), s)
 

base-commit: bec8e3c1fb91b96f337c08c6daf1bb2563b320c0

Re: [PATCH] x86/EFI: fix padding of EDD strings

[Marek Marczykowski-Górecki]

[-- Attachment #1: Type: text/plain, Size: 1221 bytes --]

On Thu, Nov 05, 2026 at 09:10:28AM +0000, Anandu Padinhare wrote:
> edd_put_string() decrements n in its copy loop condition even when the
> loop terminates because the end of the source string was reached.  The
> padding loop then writes one space too few, leaving the final byte of
> the field untouched (zero, as boot_edd_info[] lives in .bss).  On an EFI
> boot this yields e.g. a host bus type of "PCI\0" and an interface type
> of "ATAPI  \0", whereas BIOSes space-pad these fields (SeaBIOS reports
> "PCI " and "ATA     ").
> 
> Furthermore, if the string exactly filled the field, n would wrap to
> SIZE_MAX after the copy loop and the padding loop would write past the
> end of the field.  No caller currently passes such a string.
> 
> Only decrement n when a character was actually copied.  While touching
> the function, also correct its indentation.
> 
> Fixes: 51f03ec421fb ("x86-64/EFI: construct EDD data from device path protocol information")
> Assisted-by: Claude
> Signed-off-by: Anandu Padinhare <Anandu.Padinhare@electrifex.com>

Reviewed-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]