From mboxrd@z Thu Jan  1 00:00:00 1970
From: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Subject: Re: [PATCH 04/28] libxl: fix libxl_string_list_length
 and its only caller
Date: Thu, 26 Sep 2013 15:29:14 -0400
Message-ID: <52448B0A.6030808@oracle.com>
References: <1379475484-25993-1-git-send-email-mattjd@gmail.com>
	<1379475484-25993-5-git-send-email-mattjd@gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"; Format="flowed"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <1379475484-25993-5-git-send-email-mattjd@gmail.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Matthew Daley <mattjd@gmail.com>
Cc: Ian Campbell <ian.campbell@citrix.com>, Stefano Stabellini <stefano.stabellini@eu.citrix.com>, Ian Jackson <ian.jackson@eu.citrix.com>, xen-devel@lists.xen.org, David Vrabel <david.vrabel@citrix.com>
List-Id: xen-devel@lists.xenproject.org

On 09/17/2013 11:37 PM, Matthew Daley wrote:
> The wrong amount of indirections were being taken in
> libxl_string_list_length, and its only caller was miscounting the amount
> of initial non-list arguments, seemingly since the initial commit
> (599c784).
>
> This has been seen and reported in the wild (##xen):
> < Trixboxer> Hi, any idea why would I get
> < Trixboxer> xl: libxl_bootloader.c:42: bootloader_arg: Assertion `bl->nargs < bl->argsspace' failed.
> < Trixboxer> 4.2.2-23.el6
>
> Coverity-ID: 1054954
> Signed-off-by: Matthew Daley<mattjd@gmail.com>
> ---
>   tools/libxl/libxl.c            |    2 +-
>   tools/libxl/libxl_bootloader.c |    2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index 0879f23..ca24ca3 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -202,7 +202,7 @@ int libxl_string_list_length(const libxl_string_list *psl)
>   {
>       if (!psl) return 0;

This should be
     if (!psl || !(*psl)) return 0;

We segfault otherwise at the changed line below if no arguments are 
passed to the bootloader (as is the case with pvgrub).

>       int i = 0;
> -    while (*psl++) i++;
> +    while ((*psl)[i]) i++;
>       return i;
>   }

(I am surprised Coverity didn't flag this)

-boris

>   
> diff --git a/tools/libxl/libxl_bootloader.c b/tools/libxl/libxl_bootloader.c
> index ed12b2c..3287bf7 100644
> --- a/tools/libxl/libxl_bootloader.c
> +++ b/tools/libxl/libxl_bootloader.c
> @@ -48,7 +48,7 @@ static void make_bootloader_args(libxl__gc *gc, libxl__bootloader_state *bl,
>   {
>       const libxl_domain_build_info *info = bl->info;
>   
> -    bl->argsspace = 7 + libxl_string_list_length(&info->u.pv.bootloader_args);
> +    bl->argsspace = 9 + libxl_string_list_length(&info->u.pv.bootloader_args);
>   
>       GCNEW_ARRAY(bl->args, bl->argsspace);
>