xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Ian Jackson <Ian.Jackson@eu.citrix.com>,
	Matthew Daley <mattd@bugfuzz.com>,
	Ian Campbell <Ian.Campbell@citrix.com>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>,
	xen-devel@lists.xen.org
Subject: Re: [PATCH 08/13 v5] libxl: don't leak ptr in libxl_list_vm error case [and 1 more messages]
Date: Fri, 13 Dec 2013 17:05:45 +0000	[thread overview]
Message-ID: <52AB3E69.7000502@citrix.com> (raw)
In-Reply-To: <21163.15190.993775.174911@mariner.uk.xensource.com>

On 13/12/2013 16:52, Ian Jackson 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.

Can we not?

Having a non-NULL pointer to a 0 length buffer is madness, whose use
should not be further encouraged.

Furthermore, code which ends calling libxl__*alloc() with a size of 0
*is* buggy, and should suffer an abort(), just as much as attempting to
realloc to a size of 0.

>
> 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.
>
> Thanks,
> Ian.

I believe I suitably answered that question, and justified why it had to
stay.

There is an API difference between returning NULL (Call to list domains
failed), and non NULL but with nb_domains = 0 (Call to list domains
succeeded but there are no domains).

~Andrew

  reply	other threads:[~2013-12-13 17:05 UTC|newest]

Thread overview: 75+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-12-01 10:14 [PATCH 00/13] Coverity fixes for libxl Matthew Daley
2013-12-01 10:14 ` [PATCH 01/13] libxl: fix unsigned less-than-0 comparison in e820_sanitize Matthew Daley
2013-12-13  5:54   ` Matthew Daley
2013-12-13 13:23     ` Andrew Cooper
2013-12-13 17:31   ` Ian Jackson
2013-12-01 10:14 ` [PATCH 02/13] libxl: check for xc_domain_setmaxmem failure in libxl__build_pre Matthew Daley
2013-12-02 11:55   ` Ian Jackson
2013-12-02 12:11     ` [PATCH 02/13 v2] " Matthew Daley
2013-12-13  5:53       ` Matthew Daley
2013-12-13 10:17         ` Dario Faggioli
2013-12-13 17:23           ` Ian Jackson
2013-12-01 10:14 ` [PATCH 03/13] libxl: correct file open success check in libxl__device_pci_reset Matthew Daley
2013-12-02 11:57   ` Ian Jackson
2013-12-01 10:14 ` [PATCH 04/13] libxl: don't leak p in libxl__wait_for_backend Matthew Daley
2013-12-01 11:53   ` Andrew Cooper
2013-12-01 23:17     ` Matthew Daley
2013-12-02  0:27       ` [PATCH 04/13 v2] " Matthew Daley
2013-12-02  0:42         ` Andrew Cooper
2013-12-02  0:46           ` Matthew Daley
2013-12-02  0:52             ` Andrew Cooper
2013-12-02 12:00               ` Ian Jackson
2014-01-09 14:51         ` Ian Jackson
2013-12-01 10:14 ` [PATCH 05/13] libxl: remove unsigned less-than-0 comparison Matthew Daley
2013-12-02 12:05   ` Ian Jackson
2013-12-01 10:15 ` [PATCH 06/13] libxl: actually abort if initializing a ctx's lock fails Matthew Daley
2013-12-02 12:05   ` Ian Jackson
2013-12-01 10:15 ` [PATCH 07/13] libxl: don't leak output vcpu info on error in libxl_list_vcpu Matthew Daley
2013-12-02 12:05   ` Ian Jackson
2013-12-01 10:15 ` [PATCH 08/13] libxl: don't leak ptr in libxl_list_vm error case Matthew Daley
2013-12-01 12:20   ` Andrew Cooper
2013-12-02  0:30     ` Matthew Daley
2013-12-02  0:37       ` [PATCH 08/13 v2] " Matthew Daley
2013-12-02  0:39         ` Andrew Cooper
2013-12-02  2:58         ` [PATCH 08/13 v3] " Matthew Daley
2013-12-02 10:35           ` Andrew Cooper
2013-12-02 10:47             ` Matthew Daley
2013-12-02 10:50               ` Ian Campbell
2013-12-02 11:05               ` [PATCH 08/13 v4] " Matthew Daley
2013-12-02 11:10                 ` Andrew Cooper
2013-12-02 12:08                 ` Ian Jackson
2013-12-02 12:19                   ` Matthew Daley
2013-12-02 15:03                     ` Ian Jackson
2013-12-03  1:29                       ` [PATCH 08/13 v5] " Matthew Daley
2013-12-03 10:21                         ` Ian Campbell
2013-12-03 10:30                           ` Andrew Cooper
2013-12-13 16:52                           ` [PATCH 08/13 v5] libxl: don't leak ptr in libxl_list_vm error case [and 1 more messages] Ian Jackson
2013-12-13 17:05                             ` Andrew Cooper [this message]
2013-12-13 17:21                               ` Ian Jackson
2013-12-13 23:22                             ` Matthew Daley
2013-12-13 23:26                               ` Matthew Daley
2013-12-16 11:57                                 ` Ian Jackson
2013-12-14  1:15                               ` [PATCH] xl: check for libxl_list_vm failure in print_uptime Matthew Daley
2013-12-16 11:57                                 ` Ian Jackson
2013-12-16 11:58                                   ` Ian Jackson
2013-12-13  5:52                         ` [PATCH 08/13 v5] libxl: don't leak ptr in libxl_list_vm error case Matthew Daley
2013-12-01 10:15 ` [PATCH 09/13] libxl: don't leak pcidevs in libxl_pcidev_assignable Matthew Daley
2013-12-02 12:15   ` Ian Jackson
2013-12-01 10:15 ` [PATCH 10/13] libxl: don't try to fclose file twice on error in libxl_userdata_store Matthew Daley
2013-12-02 12:14   ` Ian Jackson
2013-12-02 12:24     ` Matthew Daley
2013-12-02 15:04       ` Ian Jackson
2013-12-02 23:56         ` [PATCH 10/13 v2] " Matthew Daley
2013-12-03  0:00           ` [PATCH 10/13 v3] " Matthew Daley
2013-12-03 17:28             ` Ian Jackson
2013-12-01 10:15 ` [PATCH 11/13] libxl: use pipe instead of temporary file for VNC viewer --autopass Matthew Daley
2013-12-02 12:22   ` Ian Jackson
2013-12-02 12:34     ` Matthew Daley
2013-12-01 10:15 ` [PATCH 12/13] libxl: don't leak buf in libxl_xen_console_read_start error handling Matthew Daley
2013-12-02 12:25   ` Ian Jackson
2013-12-03  1:01     ` [PATCH 12/13 v2] " Matthew Daley
2013-12-03 17:26       ` Ian Jackson
2013-12-01 10:15 ` [PATCH 13/13] libxl: replace for loop with more idiomatic do-while loop Matthew Daley
2013-12-02 12:26   ` Ian Jackson
2013-12-02 12:46     ` Matthew Daley
2013-12-01 12:22 ` [PATCH 00/13] Coverity fixes for libxl Andrew Cooper

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=52AB3E69.7000502@citrix.com \
    --to=andrew.cooper3@citrix.com \
    --cc=Ian.Campbell@citrix.com \
    --cc=Ian.Jackson@eu.citrix.com \
    --cc=mattd@bugfuzz.com \
    --cc=stefano.stabellini@eu.citrix.com \
    --cc=xen-devel@lists.xen.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).