Re: [Crash-utility] [PATCH] Fix unsigned signed comparison causing segfault for small VMCOREINFO notes
-----Original Message-----
From: Dave Anderson <anderson(a)redhat.com>
Sent: Wednesday, June 5, 2019 2:56 PM
To: Nuno Das Neves <Nuno.Das(a)microsoft.com>
Cc: Nuno Das Neves <Nuno.Das(a)microsoft.com>
Subject: Fwd: [Crash-utility] [PATCH] Fix unsigned signed comparison causing segfault for
small VMCOREINFO notes
----- Forwarded Message -----
From: "Dave Anderson" <anderson(a)redhat.com>
To: "Discussion list for crash utility usage, maintenance and development"
<crash-utility(a)redhat.com>
Sent: Wednesday, June 5, 2019 4:18:02 PM
Subject: Re: [Crash-utility] [PATCH] Fix unsigned signed comparison causing segfault for
small VMCOREINFO notes
----- Original Message -----
> Hi,
>
> This is a fix for a signed/unsigned comparison bug in vmcoreinfo_read_string.
> The bug causes a segmentation fault if size_vmcoreinfo + 1 is smaller than
> the length of the key string passed in.
I suppose that's true, but can you describe the instance where that actually
happened?
I'm debugging some issues with vm2core
(https://github.com/Azure/azure-linux-utils/tree/master/vm2core), a tool for converting
Hyper-V snapshots to elf core files that can be analyzed with Crash.
There was a problem where versions of Crash newer than 7.1.5 crashed with elf files
generated by vm2core due to this issue.
Can you show the actual note contents as shown by "help
-D"?
Elf64_Nhdr:
n_namesz: 11 ("VMCOREINFO")
n_descsz: 10
n_type: 0 (unused)
VMCOREINFO
So it seems the note doesn't really contain valid data as defined in
Documentation/kdump/vmcoreinfo.txt.
I've done some additional testing and it appears this note isn't needed by Crash
at all - I can simply patch vm2core so it doesn't add the note.
So, I suppose this fix isn't required to solve my particular issue.
However, could the VMCOREINFO note legitimately contain a single field e.g.
"PAGE_SIZE=4096"?
If so, this is just 14 characters; 14 + 1 <
strlen("NUMBER(KERNEL_IMAGE_SIZE)") - this string is used as the argument to
vmcoreinfo_read_string on line 202 of x86_64.c.
Regards,
Nuno
Thanks,
Dave
>
> Signed-off-by: Nuno Das Neves <nudasnev(a)microsoft.com>
> ---
> netdump.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/netdump.c b/netdump.c
> index 40f9cde..d257ecd 100644
> --- a/netdump.c
> +++ b/netdump.c
> @@ -1838,7 +1838,7 @@ vmcoreinfo_read_string(const char *key)
> return NULL;
>
> /* the '+ 1' is the equal sign */
> - for (i = 0; i < (size_vmcoreinfo - key_length + 1); i++) {
> + for (i = 0; i < (int)(size_vmcoreinfo - key_length + 1); i++) {
> /*
> * We must also check if we're at the beginning of VMCOREINFO
> * or the separating newline is there, and of course if we
> --
> 1.8.3.1
>
> --
> Crash-utility mailing list
> Crash-utility(a)redhat.com
>
https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.red...
utility&data=02%7C01%7CNuno.Das%40microsoft.com%7C3a91428c54b34ed7855d08d6ea008afe%7C72f988bf86f141af91ab2d7cd0
11db47%7C1%7C0%7C636953685382744097&sdata=BVID2b0QYkDmBip0bzPjX%2Fl4vltJtf78JrTE7pnqSIM%3D&reserved=0
>