Syslinux - Jan 2016 - [PATCH 2/2] core: Fix stack overflow when reloading config

On 10/12/15 21:04, celelibi--- via Syslinux wrote:
> From: Sylvain Gault <sylvain.gault at gmail.com>
> 
> The behavior when running a "CONFIG" command line is to reload
> ldlinux.c32 with the new file as argument. This call never return.
> 
> In order to avoid stacking up the calls to start_ldlinux, this patch
> introduce a setjmp/longjmp to return to the first call to start_ldlinux,
> thus freeing all the stack space.
> 
> Signed-off-by: Sylvain Gault <sylvain.gault at gmail.com>
We don't re*load* ldlinux.c32, I hope?  I assume we just re-execute it?

	-hpa

2016-01-21 21:51 UTC+01:00, H. Peter Anvin <hpa at
zytor.com>:
> On 10/12/15 21:04, celelibi--- via Syslinux wrote:
>> From: Sylvain Gault <sylvain.gault at gmail.com>
>>
>> The behavior when running a "CONFIG" command line is to
reload
>> ldlinux.c32 with the new file as argument. This call never return.
>>
>> In order to avoid stacking up the calls to start_ldlinux, this patch
>> introduce a setjmp/longjmp to return to the first call to
start_ldlinux,
>> thus freeing all the stack space.
>>
>> Signed-off-by: Sylvain Gault <sylvain.gault at gmail.com>
>
> We don't re*load* ldlinux.c32, I hope?  I assume we just re-execute it?
>
> 	-hpa
Well, ldlinux.c32 is unloaded explicitely in the function
start_ldlinux. It is then actually reloaded by a call to spawn_load.
Network capture just confirmed it.

Do you wish something else to happen?

It is, indeed, a waste of time reloading the very same file again and
again. But I hope it was made this way for simplicity.


Celelibi

On January 21, 2016 6:23:49 PM PST, Celelibi <celelibi at gmail.com>
wrote:
>2016-01-21 21:51 UTC+01:00, H. Peter Anvin <hpa at zytor.com>:
>> On 10/12/15 21:04, celelibi--- via Syslinux wrote:
>>> From: Sylvain Gault <sylvain.gault at gmail.com>
>>>
>>> The behavior when running a "CONFIG" command line is to
reload
>>> ldlinux.c32 with the new file as argument. This call never return.
>>>
>>> In order to avoid stacking up the calls to start_ldlinux, this
patch
>>> introduce a setjmp/longjmp to return to the first call to
>start_ldlinux,
>>> thus freeing all the stack space.
>>>
>>> Signed-off-by: Sylvain Gault <sylvain.gault at gmail.com>
>>
>> We don't re*load* ldlinux.c32, I hope?  I assume we just re-execute
>it?
>>
>> 	-hpa
>
>Well, ldlinux.c32 is unloaded explicitely in the function
>start_ldlinux. It is then actually reloaded by a call to spawn_load.
>Network capture just confirmed it.
>
>Do you wish something else to happen?
>
>It is, indeed, a waste of time reloading the very same file again and
>again. But I hope it was made this way for simplicity.
>
>
>Celelibi
Yes, but we should fix that.
-- 
Sent from my Android device with K-9 Mail. Please excuse brevity and formatting.

2016-01-22 3:23 UTC+01:00, Celelibi <celelibi at
gmail.com>:
> 2016-01-21 21:51 UTC+01:00, H. Peter Anvin <hpa at zytor.com>:
>> On 10/12/15 21:04, celelibi--- via Syslinux wrote:
>>> From: Sylvain Gault <sylvain.gault at gmail.com>
>>>
>>> The behavior when running a "CONFIG" command line is to
reload
>>> ldlinux.c32 with the new file as argument. This call never return.
>>>
>>> In order to avoid stacking up the calls to start_ldlinux, this
patch
>>> introduce a setjmp/longjmp to return to the first call to
start_ldlinux,
>>> thus freeing all the stack space.
>>>
>>> Signed-off-by: Sylvain Gault <sylvain.gault at gmail.com>
>>
>> We don't re*load* ldlinux.c32, I hope?  I assume we just re-execute
it?
>>
>> 	-hpa
>
> Well, ldlinux.c32 is unloaded explicitely in the function
> start_ldlinux. It is then actually reloaded by a call to spawn_load.
> Network capture just confirmed it.
>
> Do you wish something else to happen?
>
> It is, indeed, a waste of time reloading the very same file again and
> again. But I hope it was made this way for simplicity.
>
>
> Celelibi
>
BTW, this code is due to Matt Flemming with commit 3a316db1 (later
modified). The commit log say this:
    ldlinux: Loading a config file should cause re-initialisation

    There are a number of initialisation steps that need to be performed
    *every* time a config file is loaded. Reload ldlinux.c32 so that we
    can re-initialise the environment whenever a new config file is
    loaded. This involves unloading all the modules that have been loaded
    since ldlinux.c32. Luckily the list of loaded modules is sorted by
    load order, which means it's trivial to "pop" them from the
front of
    the list.
----------------------


Celelibi