[PATCH] xenguest: Add xsa-25 decompression limit prototypes

[PATCH] xenguest: Add xsa-25 decompression limit prototypes

[Andrew Cooper]

To allow xenguest consumers to also make use of the extra protection added as
a result of xsa-25.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

diff -r 5af4f2ab06f3 -r daec50a41570 tools/libxc/xenguest.h
--- a/tools/libxc/xenguest.h
+++ b/tools/libxc/xenguest.h
@@ -177,6 +177,13 @@ int xc_dom_linux_build(xc_interface *xch
 		       unsigned int console_evtchn,
 		       unsigned long *console_mfn);
 
+#ifndef XC_HAVE_DECOMPRESS_LIMITS
+#define XC_HAVE_DECOMPRESS_LIMITS
+#endif
+
+int xc_dom_kernel_max_size(struct xc_dom_image *dom, size_t sz);
+int xc_dom_ramdisk_max_size(struct xc_dom_image *dom, size_t sz);
+
 /**
  * This function will create a domain for a paravirtualized Linux
  * using buffers for kernel and initrd

Re: [PATCH] xenguest: Add xsa-25 decompression limit prototypes

[Ian Campbell]

On Fri, 2013-01-25 at 18:04 +0000, Andrew Cooper wrote:
> To allow xenguest consumers to also make use of the extra protection added as
> a result of xsa-25.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> 
> diff -r 5af4f2ab06f3 -r daec50a41570 tools/libxc/xenguest.h
> --- a/tools/libxc/xenguest.h
> +++ b/tools/libxc/xenguest.h
> @@ -177,6 +177,13 @@ int xc_dom_linux_build(xc_interface *xch
>  		       unsigned int console_evtchn,
>  		       unsigned long *console_mfn);
>  
> +#ifndef XC_HAVE_DECOMPRESS_LIMITS
> +#define XC_HAVE_DECOMPRESS_LIMITS
> +#endif

This sort of thing isn't in general needed for the libxc interface,
which already makes very little in the way of API stability guarantees.
The only existing example uses XENCTRL_HAS_* so if this is really felt
to be useful it should match.

Also this file has multiple inclusion guards so the ifndef really isn't
necessary.

> +int xc_dom_kernel_max_size(struct xc_dom_image *dom, size_t sz);
> +int xc_dom_ramdisk_max_size(struct xc_dom_image *dom, size_t sz);

These duplicate the ones in xc_dom.h, if the prototypes there aren't
sufficient then they should be moved not repeated.

> +
>  /**
>   * This function will create a domain for a paravirtualized Linux
>   * using buffers for kernel and initrd

Re: [PATCH] xenguest: Add xsa-25 decompression limit prototypes

[Andrew Cooper]

On 28/01/13 09:25, Ian Campbell wrote:
> On Fri, 2013-01-25 at 18:04 +0000, Andrew Cooper wrote:
>> To allow xenguest consumers to also make use of the extra protection added as
>> a result of xsa-25.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>
>> diff -r 5af4f2ab06f3 -r daec50a41570 tools/libxc/xenguest.h
>> --- a/tools/libxc/xenguest.h
>> +++ b/tools/libxc/xenguest.h
>> @@ -177,6 +177,13 @@ int xc_dom_linux_build(xc_interface *xch
>>  		       unsigned int console_evtchn,
>>  		       unsigned long *console_mfn);
>>  
>> +#ifndef XC_HAVE_DECOMPRESS_LIMITS
>> +#define XC_HAVE_DECOMPRESS_LIMITS
>> +#endif
> This sort of thing isn't in general needed for the libxc interface,
> which already makes very little in the way of API stability guarantees.
> The only existing example uses XENCTRL_HAS_* so if this is really felt
> to be useful it should match.
>
> Also this file has multiple inclusion guards so the ifndef really isn't
> necessary.
>
>> +int xc_dom_kernel_max_size(struct xc_dom_image *dom, size_t sz);
>> +int xc_dom_ramdisk_max_size(struct xc_dom_image *dom, size_t sz);
> These duplicate the ones in xc_dom.h, if the prototypes there aren't
> sufficient then they should be moved not repeated.

xenguest.h includes no files whatsoever, and every single prototype in
it is a duplicate of other functions in xc_*.h header files.  This
appears to be the point.  Furthermore, it is the only file referenced by
the ocaml subs for libxc, and the xenguest helper utility for Xapi.

~Andrew

>
>> +
>>  /**
>>   * This function will create a domain for a paravirtualized Linux
>>   * using buffers for kernel and initrd
>

Re: [PATCH] xenguest: Add xsa-25 decompression limit prototypes

[Ian Campbell]

On Mon, 2013-01-28 at 10:25 +0000, Andrew Cooper wrote:
> On 28/01/13 09:25, Ian Campbell wrote:
> > On Fri, 2013-01-25 at 18:04 +0000, Andrew Cooper wrote:
> >> To allow xenguest consumers to also make use of the extra protection added as
> >> a result of xsa-25.
> >>
> >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> >>
> >> diff -r 5af4f2ab06f3 -r daec50a41570 tools/libxc/xenguest.h
> >> --- a/tools/libxc/xenguest.h
> >> +++ b/tools/libxc/xenguest.h
> >> @@ -177,6 +177,13 @@ int xc_dom_linux_build(xc_interface *xch
> >>  		       unsigned int console_evtchn,
> >>  		       unsigned long *console_mfn);
> >>  
> >> +#ifndef XC_HAVE_DECOMPRESS_LIMITS
> >> +#define XC_HAVE_DECOMPRESS_LIMITS
> >> +#endif
> > This sort of thing isn't in general needed for the libxc interface,
> > which already makes very little in the way of API stability guarantees.
> > The only existing example uses XENCTRL_HAS_* so if this is really felt
> > to be useful it should match.
> >
> > Also this file has multiple inclusion guards so the ifndef really isn't
> > necessary.
> >
> >> +int xc_dom_kernel_max_size(struct xc_dom_image *dom, size_t sz);
> >> +int xc_dom_ramdisk_max_size(struct xc_dom_image *dom, size_t sz);
> > These duplicate the ones in xc_dom.h, if the prototypes there aren't
> > sufficient then they should be moved not repeated.
> 
> xenguest.h includes no files whatsoever,

I said you should move them, not change the #includes. Not that changing
the #includes isn't a good idea too.

>  and every single prototype in
> it is a duplicate of other functions in xc_*.h header files.

I don't know what tree you are looking at but that's not the case in
unstable. A random sampling of xc_linux_build, xc_linux_build_mem and
xc_await_suspend shows that they are only defined in xenguest.h.
Likewise none of the functions in xc_dom.h that I checked were defined
anywhere else but in that file.

>   This
> appears to be the point.  Furthermore, it is the only file referenced by
> the ocaml subs for libxc, 

What on earth are you talking about:
$ rgrep \#include tools/ocaml/libs/xc/
tools/ocaml/libs/xc/xenctrl_stubs.c:#include <stdlib.h>
tools/ocaml/libs/xc/xenctrl_stubs.c:#include <errno.h>
tools/ocaml/libs/xc/xenctrl_stubs.c:#include <caml/alloc.h>
tools/ocaml/libs/xc/xenctrl_stubs.c:#include <caml/memory.h>
tools/ocaml/libs/xc/xenctrl_stubs.c:#include <caml/signals.h>
tools/ocaml/libs/xc/xenctrl_stubs.c:#include <caml/fail.h>
tools/ocaml/libs/xc/xenctrl_stubs.c:#include <caml/callback.h>
tools/ocaml/libs/xc/xenctrl_stubs.c:#include <sys/mman.h>
tools/ocaml/libs/xc/xenctrl_stubs.c:#include <stdint.h>
tools/ocaml/libs/xc/xenctrl_stubs.c:#include <string.h>
tools/ocaml/libs/xc/xenctrl_stubs.c:#include <xenctrl.h>
tools/ocaml/libs/xc/xenctrl_stubs.c:#include "mmap_stubs.h"

Not only is it not the only file referenced, it's not even referenced at
all!

> and the xenguest helper utility for Xapi.

Possibly, but that doesn't preclude fixing that if that is the right
thing to do.

Ian.