[syslinux] [PATCH 2/2] core: Fix stack overflow when reloading config

[Ady]

> >I see 3 ways of handling this.
> >1) Have some specific code in ldlinux.c32 that handles
> >reinitialization.
> >2) Have some specific cache for the COM32 modules and load them only
> >once for the lifetime of the whole boot loader.
> >3) Put a file system cache that would also benefit to other files.
> >
> >I would tend to prefer the third way, but I don't know how much work
> >it would be or if it would integrate well in the current design.
> >
> >
> >Celelibi
> 
> #2 is really easy; we just need to keep a copy of the unmodified
> data section.  The only risk with this approach is running out of
> memory, but I think that risk is minor. -- 
 
 
Please forgive my lack of technical knowledge about the code itself.

Regarding "...running out of memory... the risk is minor."

This "wishful thinking" reminds me of a different case we had; 
although, code-wise, it was not a similar problem (I would assume).

A patch from Shao, "fs: Fix searchdir resource leak", committed 
2012Nov,

_ for 5.xx commit 57acc34bdf83fc5ea08dbf44b74a5dd2c1131187

_ for 4.xx commit 37971728a5fc40b1c90512e79e47333d98ec8851

reads:
 *** 
This is a significant rewrite of the generic lookup logic inside 
core/fs/fs.c's searchdir function.  Previously, there was a memory leak 
if a path involved multiple directories.  After a sufficiently large 
number of invocations, this could be observed.
 *** 

The code, before that patch, used to assume that just "a few" files 
were involved. A c32 module that has to deal with many more files 
showed the leak, and the crash.

(BTW, I am still waiting - after more than 3 years - and hoping for 
that useful c32 module to be part of official Syslinux.)

Additionally, the amount of available resources (including memory, 
among others) in some cases is not _that_ high, and we frequently 
forget they exist. Cases such as the DOS-based installer comes to mind, 
and the length of the file names for library modules (e.g. 
"libutil_com.c32") is another one. It was _that_ easy to forget that 
FAT and ISO9660 do not support such file names (thus, a library module 
shall not use unsupported file names), and it is easy for anyone to 
assume that "his own" use-case is broad-enough that it covers anyone's 
/ everyone's else.

It is also not a coincidence that we already have seen leaks related to 
directives such as CONFIG and INCLUDE in the past.

I hope we are not introducing more problems while we are trying to 
solve one.

Regards,
Ady.