[Libguestfs] [PATCH v2 1/2] appliance: search all types of appliances for each path separately

[Pavel Butsykin]

On 05.05.2017 12:27, Richard W.M. Jones wrote:
>
> Looks good.  I'll push this if it passes 'make check && make check-valgrind'
> which I'm currently running.

Thanks.


It is not connected with the current patch-set, but I noticed a
possible bug in appliance.c:

static int
dir_contains_files (guestfs_h *g, const char *dir, ...)
{
   va_list args;
   const char *file;

   va_start (args, dir);
   while ((file = va_arg (args, const char *)) != NULL) {
     if (!dir_contains_file (g, dir, file)) {
...

static int
contains_fixed_appliance (guestfs_h *g, const char *path, void *data)
{
   return dir_contains_files (g, path,
                              "README.fixed",
                              "kernel", "initrd", "root", NULL);
}

Passing NULL as the last argument to dir_contains_file() can leads
to undefined behavior in accordance with C99.
7.15.1.1
If there is no actual next argument, or if type is not compatible with
the type of the actual next argument (as promoted according to the
default argument promotions), the behavior is undefined, except for
the following cases:
— one type is a signed integer type, the other type is the
   corresponding unsigned integer type, and the value is representable
   in both types;
— one type is pointer to void and the other is a pointer to a
   character type

There NULL is macros which can be defined as 0 or (void*)0, again in
accordance with c99:

7.17 Common definitions
..
The macros are NULL which expands to an implementation-defined null
pointer constant;

6.3.2.3 Pointers
...
3 An integer constant expression with the value 0, or such an
expression cast to type void *, is called a null pointer constant. If a
null pointer constant is converted to a pointer type, the resulting
pointer, called a null pointer, is guaranteed to compare unequal to a
pointer to any object or function.


In practice, this means that if NULL is defined as integer and we have
64 bit architecture, then here as the last argument - 4 bytes are
written on the stack:
dir_contains_files (g, path,
                     "README.fixed", "kernel", "initrd", "root", NULL);

But va_arg() will read 8 bytes:
	while ((file = va_arg (args, const char *)) != NULL) {

I don't know how real can be the conditions to reproduce this, I mean
the standard header where #define NULL (0), but as an extra precaution
I can offer a patch (attached).

> Rich.
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-applaince-iterable-args-should-be-compatible-with-ty.patch
Type: text/x-patch
Size: 1534 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20170505/f19b1b2c/attachment.bin>