> -----Original Message----- > From: Dave Anderson <anderson@xxxxxxxxxx> > Sent: Wednesday, June 5, 2019 2:56 PM > To: Nuno Das Neves <Nuno.Das@xxxxxxxxxxxxx> > Cc: Nuno Das Neves <Nuno.Das@xxxxxxxxxxxxx> > Subject: Fwd: [PATCH] Fix unsigned signed comparison causing segfault for small VMCOREINFO notes > > > > ----- Forwarded Message ----- > From: "Dave Anderson" <anderson@xxxxxxxxxx> > To: "Discussion list for crash utility usage, maintenance and development" <crash-utility@xxxxxxxxxx> > Sent: Wednesday, June 5, 2019 4:18:02 PM > Subject: Re: [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@xxxxxxxxxxxxx> > > --- > > 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@xxxxxxxxxx > > https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.redhat.com%2Fmailman%2Flistinfo%2Fcrash- > utility&data=02%7C01%7CNuno.Das%40microsoft.com%7C3a91428c54b34ed7855d08d6ea008afe%7C72f988bf86f141af91ab2d7cd0 > 11db47%7C1%7C0%7C636953685382744097&sdata=BVID2b0QYkDmBip0bzPjX%2Fl4vltJtf78JrTE7pnqSIM%3D&reserved=0 > > -- Crash-utility mailing list Crash-utility@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/crash-utility
