On 03/09/2010 04:31 PM, Eric Blake wrote:
> 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.
Gah, I looked at it backwards. You are right of course. Still, we
might want to verify that res.length is valid.
>
>> 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?
This is a good point too. I don't know the answer.
>
>> 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?
I know danpb originally wrote the code, and does use it. Whether libvirt-tck
exercises it is unknown to me.
--
Chris Lalancette
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list