Re: [Xen-devel] [PATCH 08/13 v5] libxl: don't leak ptr in libxl_list_vm error case [and 1 more messages]

[Matthew Daley <mattd@xxxxxxxxxxx>]

On Sat, Dec 14, 2013 at 5:52 AM, Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx> wrote:
> Ian Campbell writes ("Re: [PATCH 08/13 v5] libxl: don't leak ptr in 
> libxl_list_vm error case"):
>> On Tue, 2013-12-03 at 14:29 +1300, Matthew Daley wrote:
>> > +    /*
>> > +  * Always make sure to allocate at least one element; if we don't and we
>> > +  * request zero, libxl__calloc (might) think its internal call to calloc
>> > +  * has failed (if it returns null), if so it would kill our process.
> [rewrapped -iwj]
>>
>> Is size==0 something we could/should handle in our libxl__*alloc
>> wrappers?
>
> I think so.  I think they should promise that if you pass size==0 you
> get a non-null pointer.  Calling realloc with size==0 should crash.
>
> Matthew Daley writes ("Re: [PATCH 08/13 v5] libxl: don't leak ptr in 
> libxl_list_vm error case"):
>> Ping?
>
> See Ian C's comment above, which AFAICT hasn't been answered.

Sorry, I implicitly agreed with Andrew's comment (at least, the second part...)

For the record, for custom wrapper functions like these I usually
prefer to stick as close to the existing functions' semantics as
possible, not so much out of love for standards but out of desiring
the absence of surprising behaviour.

As for the second part of the comment, as Andrew already mentioned,
the nb_domains output argument is sufficient to distinguish failure
vs. 0 domains from libxl_list_vm already.

Out of curiosity, looking through libxl_list_vm's existing callers:
- libxl__domain_make is only (ab)using libxl_list_vm to get the number
of vms via nb_domains... it just frees the vm list right afterward!
- xl_cmdimpl.c:print_uptime doesn't even check libxl_list_vm's result
:D  I will patch this shortly.

(Also, I see you have applied my patch regardless of this discussion; thanks.)

- Matthew

>
> Thanks,
> Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel