Re: [PATCH] target/s390x/arch_dump: Fix memory corruption in s390x_write_elf64_notes()

[Janosch Frank]

On 2/14/23 15:10, Thomas Huth wrote:
> "note_size" can be smaller than sizeof(note), so unconditionally calling
> memset(notep, 0, sizeof(note)) could cause a memory corruption here in
> case notep has been allocated dynamically, thus let's use note_size as
> length argument for memset() instead.
>
> Fixes: 113d8f4e95 ("s390x: pv: Add dump support")
> Signed-off-by: Thomas Huth <thuth@redhat.com>

This was found because a machine was used that has PV support but 
doesn't have the PV dump support (I currently don't have access to a 
previous generation machine). In that case the size of the PV cpu note 
is reported as 0 but it's still being written / processed.

I added proper checks for dump support on my todo list so we can avoid 
writing empty notes. However it's easier said than done since "dump 
support" is actually a combination of KVM, QEMU, the machine AND a bit 
in the SE header that allows dumping. Additionally we need to report the 
size of the notes way before we start the PV dump process where we get 
told if the machine is allowed to dump.

Thanks for helping with the debug effort and creating a patch Thomas!

Reviewed-by: Janosch Frank <frankja@linux.ibm.com>

Also:
Reported-by: Sebastian Mitterle <smitterl@redhat.com>

> ---
>   target/s390x/arch_dump.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/s390x/arch_dump.c b/target/s390x/arch_dump.c
> index a2329141e8..a7c44ba49d 100644
> --- a/target/s390x/arch_dump.c
> +++ b/target/s390x/arch_dump.c
> @@ -248,7 +248,7 @@ static int s390x_write_elf64_notes(const char *note_name,
>               notep = g_malloc(note_size);
>           }
>   
> -        memset(notep, 0, sizeof(note));
> +        memset(notep, 0, note_size);
>   
>           /* Setup note header data */
>           notep->hdr.n_descsz = cpu_to_be32(content_size);