On 03/09/2010 12:17 PM, Chris Lalancette wrote: >> if (nbytes < sizeof res) { >> - virReportSystemError(errno, >> - _("incomplete reply %s"), >> - cmd); >> + virReportSystemError(0, _("incomplete reply %s"), cmd); >> + goto error; >> + } >> + if (sizeof res.data < res.length) { >> + virReportSystemError(0, _("invalid length in reply %s"), cmd); >> goto error; >> } > > Let me preface this by saying that this is the first time I've ever looked at > UML. I'm with you in the fresh eyes category on this one :) > With that being said, I'm not sure this above check is correct. > sizeof(res.data) is always a constant 512, but res.length may or may not vary > based on the message. That is, it looks to me like it's perfectly valid > for res.length to be less than res.data. That is a true statement. But this check is whether res.length is _larger_ than 512, in which case the packet is bogus. > In point of fact the code in > uml_mconsole.c (which is where this was ported from) ignores res.length altogether > and just uses strlen(res.data) to realloc and copy. Possibly a valid approach, but the version in this patch was the one recommended by Jim. For that matter, is strlen(res.data) safe, or can it run past the end of res.length bytes if the user (maliciously) failed to pass in a NUL terminator? > I'd be wary of pushing > this code without testing it out under UML. All right then - any advice from someone who HAS used UML on how to test this beyond mere code inspection? Related question: does libvirt-tck exercise UML? -- Eric Blake eblake@xxxxxxxxxx +1-801-349-2682 Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list