Re: [PATCH for-5.0] dump: Fix writing of ELF section

["Marc-André Lureau" <marcandre.lureau@redhat.com>]

Hi

On Tue, Mar 24, 2020 at 6:36 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> In write_elf_section() we set the 'shdr' pointer to point to local
> structures shdr32 or shdr64, which we fill in to be written out to
> the ELF dump.  Unfortunately the address we pass to fd_write_vmcore()
> has a spurious '&' operator, so instead of writing out the section
> header we write out the literal pointer value followed by whatever is
> on the stack after the 'shdr' local variable.
>
> Pass the correct address into fd_write_vmcore().
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


> ---
> I have not tested this because I can't reproduce the conditions
> under which we try to actually use write_elf_section() (they
> must be rare, because currently we produce a bogus ELF file
> for this code path). In dump_init() s->list.num must be
> at least UINT16_MAX-1, which I think means it has to be a
> paging-enabled dump and the guest's page table must be
> extremely fragmented ?

yeah, I can't help either without spending more time playing with it,
but the fix looks good nonetheless.

> ---
>  dump/dump.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/dump/dump.c b/dump/dump.c
> index 6fb6e1245ad..22ed1d3b0d4 100644
> --- a/dump/dump.c
> +++ b/dump/dump.c
> @@ -364,7 +364,7 @@ static void write_elf_section(DumpState *s, int type, Error **errp)
>          shdr = &shdr64;
>      }
>
> -    ret = fd_write_vmcore(&shdr, shdr_size, s);
> +    ret = fd_write_vmcore(shdr, shdr_size, s);
>      if (ret < 0) {
>          error_setg_errno(errp, -ret,
>                           "dump: failed to write section header table");
> --
> 2.20.1
>