Syslinux - Feb 2014 - [PATCH] efi: off-by-one in gdt allocation

From: Sylvain Gault <sylvain.gault at gmail.com>

The assembly instruction lgdt take a segment limit that is one less than
the actual size, so that base+limit points to the last byte.

Signed-off-by: Sylvain Gault <sylvain.gault at gmail.com>
---
 efi/main.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/efi/main.c b/efi/main.c
index 94878f9..bdf9353 100644
--- a/efi/main.c
+++ b/efi/main.c
@@ -450,7 +450,7 @@ struct dt_desc {
 	uint64_t *base;
 } __packed;
 
-struct dt_desc gdt = { 0x800, (uint64_t *)0 };
+struct dt_desc gdt = { 0x7ff, (uint64_t *)0 };
 struct dt_desc idt = { 0, 0 };
 
 static inline EFI_MEMORY_DESCRIPTOR *
@@ -852,12 +852,12 @@ static int build_gdt(void)
 	EFI_STATUS status;
 
 	/* Allocate gdt consistent with the alignment for architecture */
-	status = emalloc(gdt.limit, __SIZEOF_POINTER__ , (EFI_PHYSICAL_ADDRESS
*)&gdt.base);
+	status = emalloc(gdt.limit + 1, __SIZEOF_POINTER__ , (EFI_PHYSICAL_ADDRESS
*)&gdt.base);
 	if (status != EFI_SUCCESS) {
 		printf("Failed to allocate memory for GDT, bailing out\n");
 		return -1;
 	}
-	memset(gdt.base, 0x0, gdt.limit);
+	memset(gdt.base, 0x0, gdt.limit + 1);
 
 	/*
          * 4Gb - (0x100000*0x1000 = 4Gb)
-- 
1.8.5.3

On 02/21/2014 06:30 PM, celelibi at gmail.com wrote:
> From: Sylvain Gault <sylvain.gault at gmail.com>
>
> The assembly instruction lgdt take a segment limit that is one less than
> the actual size, so that base+limit points to the last byte.
>
> Signed-off-by: Sylvain Gault <sylvain.gault at gmail.com>
This patch is of course correct (and will be applied); the bug is real, 
but it is (fortunately) harmless, as the extra byte at the end of the 
GDT cannot actually be accessed since it doesn't fit a full descriptor.

	-hpa

2014-02-22 9:05 UTC+01:00, H. Peter Anvin <hpa at
zytor.com>:
> On 02/21/2014 06:30 PM, celelibi at gmail.com wrote:
>> From: Sylvain Gault <sylvain.gault at gmail.com>
>>
>> The assembly instruction lgdt take a segment limit that is one less
than
>> the actual size, so that base+limit points to the last byte.
>>
>> Signed-off-by: Sylvain Gault <sylvain.gault at gmail.com>
>
> This patch is of course correct (and will be applied); the bug is real,
> but it is (fortunately) harmless, as the extra byte at the end of the
> GDT cannot actually be accessed since it doesn't fit a full descriptor.
>
> 	-hpa
Yes, I know. I just found this while hunting another bug and thought
this was worth a patch.


Celelibi