[syslinux] Proposed fix: syslinux

[Luiz Carlos Ramos]

On Fri, Sep 29, 2023 at 05:25:32PM -0700, H. Peter Anvin wrote:
> On September 29, 2023 5:01:50 PM PDT, Luiz Carlos Ramos <lramos.prof at yahoo.com.br> wrote:
> >Hello,
> >
> >possibly I expressed myself badly. What happens is that the exit_boot()
> >routine takes a lot of "memory buffers" and consolidate them in
> >contiguous "segments" and I couldn't find a clear wording for these two
> >things.
> >
> >I referred to those "memory buffers" as "chunks" in the former email,
> >and I agree that this may not be the better word for it. Anyway, they
> >were more than 190, and this range was that ultimately brought the
> >problem into life, as E820MAX is set to 128 and it is taken as a limit
> >for the iteration counter.
> >
> >The "segments" (again, not the better word) are about 20, and so
> >dimensioning E820MAX to 128 seems to be ok.
> >
> >The real issue is that the loop used the same index for both things,
> >that is, input and output tables.
> >
> >Thanks for your quick reply! Please don't hesitate in reaching me back
> >in case of any imprecisions in my text.
> >Luiz
> >
> >
> >On Fri, Sep 29, 2023 at 04:10:28PM -0700, H. Peter Anvin wrote:
> >> On September 29, 2023 3:57:04 PM PDT, Luiz Carlos Ramos <lramos.prof at yahoo.com.br> wrote:
> >> >Hello,
> >> >
> >> >my name is Luiz Ramos. I work in Brazil and causually had to pay
> >> >attention to some bits of the Syslinux project, ultimately having found
> >> >what I think it's a bug.
> >> >
> >> >The problem shows itself as an inability of the loaded Linux kernel
> >> >to access all the memory the machine has. This is due to the fact that
> >> >there is an issue in the exit_boot() routine, that prevents it from
> >> >searching for memory chunks above id 128 (i.e., E820MAX).
> >> >
> >> >Below I propose a patch which hopefully solves the problem (or at least,
> >> >solved my case):
> >> >
> >> >=== patch ============================================================
> >> >diff --git a/efi/main.c b/efi/main.c
> >> >index 6a748412..ce8f21ec 100644
> >> >--- a/efi/main.c
> >> >+++ b/efi/main.c
> >> >@@ -1034,7 +1034,7 @@ static int exit_boot(struct boot_params *bp)
> >> > 	 * interim.
> >> > 	 */
> >> > 	e = e820buf = bp->e820_map;
> >> >-	for (i = 0; i < nr_entries && i < E820MAX; i++) {
> >> >+	for (i = 0; i < nr_entries; i++) {
> >> > 		struct e820_entry *prev = NULL;
> >> > 
> >> > 		if (e > e820buf)
> >> >@@ -1083,8 +1083,11 @@ static int exit_boot(struct boot_params *bp)
> >> > 		if (prev && (prev->start + prev->len) == e->start &&
> >> > 		    prev->type == e->type)
> >> > 			prev->len += e->len;
> >> >-		else
> >> >+		else {
> >> > 			e++;
> >> >+			if (e > &e820_type[E820MAX-1])
> >> >+				break;
> >> >+		}
> >> > 	}
> >> > 
> >> > 	bp->e820_entries = e - e820buf;
> >> >=== patch ============================================================
> >> >
> >> >Hope it may help,
> >> >Luiz Carlos Ramos
> >> >lramos.prof at yahoo.com.br
> >> >
> >> 
> >> Yikes. What machine do you have that has more than 128 chunks and actually need them to boot?
> 
> It is ok, I'm trying to figure out what system you are working with and what your memory map looks like.
> 
> This is probably a carryover from the BIOS code, for which a large number of memory regions is extremely rare, and because of the limitations of the real-mode environment it is assumed that the first 128 entries is enough for the boot process to complete (the kernel itself has no such limit anymore.)
> 

Understood. The system is not exotic; it's a kind of a PC dedicated to a
specific task.

When I say about a number of memory regions, that's what I learned
checking some logs/printfs in exit_boot(). I saw a lot of "unused"
items aside a number of active ones, but that were very small as a
general rule (by small I mean about ~60 bytes). It seems for me that
they are the final stage of a lot of memory allocations done before.

Anyway, the loop counter seems to be designed for iterate for the input
table only. So that counter should not be compared with the size of the
output table.

Many thanks,
Luiz