[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 04/28] libxl: fix libxl_string_list_length and its only caller
On 26/09/13 20:29, Boris Ostrovsky wrote: > 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@xxxxxxxxx> >> --- >> 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 For this bug, Coveritys specific objection was that the caller passes a singleton pointer into this function, and this function uses it as an array. This is a side effect of using the wrong indirection. However, your spot of the crash is good, and needs fixing as well. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |