[PATCH 1/1] xen/efi: Config parsing: Free the same page count as allocated.

[Bernhard Kaindl <bernhard.kaindl@xxxxxxxxxx>]

When reading the config file or the config section, we may need to append
a terminating NUL byte to the config buffer if the last byte is not a
control character.

This may cause an additional page to be allocated by efi_bs->AllocatePages()
if the config file or config section size is a multiple of the page size.

For this case, increment file->size by one so the number of pages to be
freed by efi_bs->FreePages() is the same as the number of pages allocated
by efi_bs->AllocatePages() when the additional byte is allocated.

I moved the dcache flush after the NUL termination so the flushed range
covers the final buffer contents.

I didn't add a dcache flush for the copied cfg buffer in read_section():
that buffer is created by memcpy() and then consumed only by normal CPU
reads in the EFI loader, so there is no non-coherent producer or other
observer that would require cache maintenance.

Fixes: df75f77092c1 ("EFI: avoid OOB config file reads")
Signed-off-by: Bernhard Kaindl <bernhard.kaindl@xxxxxxxxxx>

----
PS: The same fix applies to its backport to 4.21 and other branches,
for example:

stable-4.21:
Fixes: feb99494bf59 ("EFI: avoid OOB config file reads")

stable-4.20:
Fixes: 05b8f716aa32 ("EFI: avoid OOB config file reads")

PPS: A review using GPT-5.4 (just a data point for review) confirms what
I've found by manual code inspection:

> The fix in boot.c:882-886 and boot.c:913-928 is consistent with the existing
> free sites at boot.c:790-797, boot.c:1564-1567, and boot.c:1638-1641: Once the
> config buffer gets an extra terminating byte, using the incremented size for
> PFN_UP during FreePages is the right fix.
>
> I also checked the parser helpers at boot.c:584-641; they already operate on
> a bounded buffer and treat NUL/control bytes as terminators, so the synthetic
> extra byte does not create an obvious parsing regression.
---
 xen/common/efi/boot.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index b44762878be4..94461b113180 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -858,7 +858,8 @@ static bool __init read_file(EFI_FILE_HANDLE dir_handle, 
CHAR16 *name,
     what = L"Allocation";
     file->addr = min(1UL << (32 + PAGE_SHIFT),
                      HYPERVISOR_VIRT_END - DIRECTMAP_VIRT_START);
-    /* For config files allocate an extra byte to put a NUL there. */
+
+    /* For config file buffers, allocate space for the terminating NUL byte */
     ret = efi_bs->AllocatePages(AllocateMaxAddress, EfiLoaderData,
                                 PFN_UP(size + (file == &cfg)), &file->addr);
     if ( EFI_ERROR(ret) )
@@ -877,10 +878,12 @@ static bool __init read_file(EFI_FILE_HANDLE dir_handle, 
CHAR16 *name,
 
     FileHandle->Close(FileHandle);
 
-    efi_arch_flush_dcache_area(file->ptr, file->size);
-
     if ( file == &cfg )
-        file->str[file->size] = 0;
+    {
+        file->str[file->size] = 0; /* NUL-terminate the config data buffer. */
+        file->size += 1;           /* Free the same page count as allocated. */
+    }
+    efi_arch_flush_dcache_area(file->ptr, file->size);
 
     return true;
 
@@ -907,9 +910,9 @@ static bool __init read_section(const EFI_LOADED_IMAGE 
*image,
 
     file->ptr = ptr;
 
-    /* For cfg file, if necessary allocate space to put an extra NUL there. */
     if ( file == &cfg && file->size && !iscntrl(file->str[file->size - 1]) )
     {
+        /* Create a copy the config section for terminating the config buffer 
*/
         EFI_PHYSICAL_ADDRESS addr;
         EFI_STATUS ret = efi_bs->AllocatePages(AllocateMaxAddress,
                                                EfiLoaderData,
@@ -922,6 +925,7 @@ static bool __init read_section(const EFI_LOADED_IMAGE 
*image,
         file->addr = addr;
         file->need_to_free = true;
         file->str[file->size] = 0;
+        file->size += 1; /* Free the same page count as allocated. */
     }
 
     handle_file_info(name, file, options);
-- 
2.39.5