Re: zmodload (-u?) changing options (was Re: [PATCH] Named reference typos & misc.)

[Peter Stephenson <p.w.stephenson@xxxxxxxxxxxx>]

> On 16/02/2023 18:17 Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx> wrote:
> On Thu, Feb 16, 2023 at 7:53 AM Peter Stephenson
> <p.w.stephenson@xxxxxxxxxxxx> wrote:
> >
> > Might be something valgrind could help with, it's hard to see how
> > this could be deliberate even as an obscure side effect.
> 
> I rigged up a script as Src/zsh so that I could invoke valgrind from
> the test harness.  Got multiple invalid reads for this block:
> 
> ==746398==  Block was alloc'd at
> ==746398==    at 0x483B7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgprel
> oad_memcheck-amd64-linux.so)
> ==746398==    by 0x190208: zalloc (mem.c:966)
> ==746398==    by 0x1903B9: zshcalloc (mem.c:979)
> ==746398==    by 0x19B2CD: createparam (params.c:1061)
> ==746398==    by 0x1A1048: assignsparam (params.c:3130)
> ==746398==    by 0x1A1D02: setsparam (params.c:3240)
> ==746398==    by 0x19270F: add_autoparam (module.c:1215)
> ==746398==    by 0x19862A: autofeatures (module.c:3612)
> ==746398==    by 0x19686F: unload_module (module.c:2902)
> ==746398==    by 0x196A04: unload_named_module (module.c:2949)
> ==746398==    by 0x196ABD: bin_zmodload_load (module.c:2971)
> ==746398==    by 0x1957AD: bin_zmodload (module.c:2499)

This means that when we unloaded the module we restored a list
of autoloadable parameters so it could be automatically reloaded.
However, someone has subsequently freed the memory associated
with that autoloadable parameter stub...
 
> ==746398==  Address 0x643b030 is 16 bytes inside a block of size 80 free'd
> ==746398==    at 0x483CA3F: free (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreloa
> d_memcheck-amd64-linux.so)
> ==746398==    by 0x1907C9: zfree (mem.c:1871)
> ==746398==    by 0x1A80C3: freeparamnode (params.c:5818)
> ==746398==    by 0x1A4681: unsetparam_pm (params.c:3779)
> ==746398==    by 0x19206F: checkaddparam (module.c:1052)
> ==746398==    by 0x19209A: addparamdef (module.c:1065)
> ==746398==    by 0x192463: setparamdefs (module.c:1174)
> ==746398==    by 0x197E48: setfeatureenables (module.c:3367)
> ==746398==    by 0x197EC3: handlefeatures (module.c:3385)
> ==746398==    by 0x1F7CC5: enables_zshQsparameter (parameter.c:2333)
> ==746398==    by 0x193164: enables_module (module.c:1948)
> ==746398==    by 0x1936C6: do_module_features (module.c:2109)

This is a normal module load sequence.  I would imagine what it's
doing is taking out the parameters marked as autoloads because it's
putting in real parameters instead.

The point where the invalid memory is being accessed is therefore
apparently using an autoload stub when the full parameter has
already been loaded.  So is the code in question hanging onto
a parameter pointer that it should instead be looking up again
after some complicated intervening operation that happens to do
the autoload?

pws