Re: [Info-mtools] segfault in 4.0.29 mcopy

[Alain Knaff]

Hi,

On 07/06/2021 13:48, Natanael Copa wrote:
[...]
> I don't think it is. I believe the command line from the script was:
> 
>   mcopy -i ${DESTDIR}/boot/grub/efi.img -s ${DESTDIR}/efi ::
> 
> From here: 
> https://gitlab.alpinelinux.org/alpine/aports/-/blob/master/scripts/mkimg.base.sh#L253

ok, good.

[...]
> I was able to reproduce it with:
> 
>   mformat -i /tmp/efi.img -C -f 1440
>   mcopy -i /tmp/efi.img /etc/issue ::
>   Segmentation fault

good

[...]
>> Did you use any other compilation flags which might help me reproduce 
>> this?
> 
> From build log:
> 
> gcc -Os -fomit-frame-pointer -DHAVE_CONFIG_H -DSYSCONFDIR=\"/etc/mtools\" 
> -DCPU_i586 -DVENDOR_alpine -DOS_linux_musl -Os -fomit-frame-pointer -g -Wall 
> -fno-strict-aliasing -I.  -I.  -c strtonum.c                                  
>                               
> gcc -Os -fomit-frame-pointer -DHAVE_CONFIG_H -DSYSCONFDIR=\"/etc/mtools\" 
> -DCPU_i586 -DVENDOR_alpine -DOS_linux_musl -Os -fomit-frame-pointer -g -Wall 
> -fno-strict-aliasing -I.  -I.  -c mkmanifest.c -Os
> 
> So compiler flags are -Os -fomit-frame-pointer

Good.

> 
> Another thing I discovered is that -DOS_linux_musl does not set the OS_linux 
> define, which I think it should.

Does indeed not look so good :-(

[...]
> I have experimented a bit with it and it seems like I am not able to
> reproduce it when I build without -fomit-frame-pointer. Here is another 
> backtrace without optimizations (but with -fomit-frame-pointer):

That, or disabling XDF (see below)

[...]
> Here is the output from valgrind:
> ==37442== Memcheck, a memory error detector
> ==37442== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
> ==37442== Using Valgrind-3.17.0 and LibVEX; rerun with -h for copyright info
> ==37442== Command: ./mcopy -i /tmp/efi.img /etc/issue ::
> ==37442== 
> ==37442== Conditional jump or move depends on uninitialised value(s)
> ==37442==    at 0x11623C: try_device (init.c:181)

Indeed, if XDF is disabled, Stream is uninitialized in line 181 of
init.c (it would have been initialized by XdfOpen in line 172, if XDF
was activated).

The fix here is to add initialization in line 159:

                Stream_t *Stream=NULL;

And this missing initialization could indeed lead to random values in
Stream.

Will be fixed in 4.0.30

Regards,

Alain