[PATCH] stubdom/libxc: Fix build of unsafe decompressors

[PATCH] stubdom/libxc: Fix build of unsafe decompressors

[Andrew Cooper]

c/s b683d68c386 changed the packing attribute on struct lzma_header.  libxc
includes the Xen decompression .c, (for stubdoms only) but without access to
the Xen header files.

Fix in the main header file for any potential change in Xen, rather than in
for just lzma alone.

Reported-by: Sander Eikelenboom <linux@eikelenboom.it>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Keir Fraser <keir@xen.org>
CC: Jan Beulich <JBeulich@suse.com>
CC: Ian Campbell <Ian.Campbell@citrix.com>
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 tools/libxc/xc_dom_decompress_unsafe.h |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/libxc/xc_dom_decompress_unsafe.h b/tools/libxc/xc_dom_decompress_unsafe.h
index 64f6886..66124a8 100644
--- a/tools/libxc/xc_dom_decompress_unsafe.h
+++ b/tools/libxc/xc_dom_decompress_unsafe.h
@@ -1,5 +1,7 @@
 #include "xc_dom.h"
 
+#define __packed __attribute__((packed))
+
 typedef int decompress_fn(unsigned char *inbuf, unsigned int len,
                           int (*fill)(void*, unsigned int),
                           int (*flush)(void*, unsigned int),
-- 
1.7.10.4

Re: [PATCH] stubdom/libxc: Fix build of unsafe decompressors

[Ian Campbell]

On Fri, 2014-03-14 at 11:16 +0000, Andrew Cooper wrote:
> c/s b683d68c386 changed the packing attribute on struct lzma_header.

Since this is third party code (which presumably we want to keep mostly
in sync) was this a good idea? Perhaps we should just revert that
portion?

>   libxc
> includes the Xen decompression .c, (for stubdoms only) but without access to
> the Xen header files.
> 
> Fix in the main header file for any potential change in Xen, rather than in
> for just lzma alone.

This has the disadvantage that any external compression we import in the
future had better not define it's own __packed macro.

Also __packed is not validly namespaced for userspace where it is
reserved for either POSIX or the compiler (can't be bother to check
which). Maybe we already get this wrong in other places but given the is
change to 3rd party code I'm even more strongly inclined to go with the
revert option.

Ian.

> 
> Reported-by: Sander Eikelenboom <linux@eikelenboom.it>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> CC: Keir Fraser <keir@xen.org>
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Ian Campbell <Ian.Campbell@citrix.com>
> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
> ---
>  tools/libxc/xc_dom_decompress_unsafe.h |    2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/tools/libxc/xc_dom_decompress_unsafe.h b/tools/libxc/xc_dom_decompress_unsafe.h
> index 64f6886..66124a8 100644
> --- a/tools/libxc/xc_dom_decompress_unsafe.h
> +++ b/tools/libxc/xc_dom_decompress_unsafe.h
> @@ -1,5 +1,7 @@
>  #include "xc_dom.h"
>  
> +#define __packed __attribute__((packed))
> +
>  typedef int decompress_fn(unsigned char *inbuf, unsigned int len,
>                            int (*fill)(void*, unsigned int),
>                            int (*flush)(void*, unsigned int),

Re: [PATCH] stubdom/libxc: Fix build of unsafe decompressors

[Andrew Cooper]

On 14/03/14 11:23, Ian Campbell wrote:
> On Fri, 2014-03-14 at 11:16 +0000, Andrew Cooper wrote:
>> c/s b683d68c386 changed the packing attribute on struct lzma_header.
> Since this is third party code (which presumably we want to keep mostly
> in sync) was this a good idea?

It seemed so at the time

> Perhaps we should just revert that portion?

That might be be the easiest option.

Frankly, my opinion is that anything under xen/ is fair game for having
hypervisor style and consistency applied to it.  It should be reasonable
to expect that any code under xen/ is exclusive to the hypervisor, and
the real issue is having library code not obviously separated.

v2 patch on its way.

~Andrew

[PATCH] xen/unlzma: Fix build of stubdom unsafe decompressors

[Andrew Cooper]

c/s b683d68c386 changed the packing attribute on struct lzma_header.  However,
this is 3rd library code used by stubdomains.

Revert the change to lzma_header alone to avoid needless divergence from its
source.

Reported-by: Sander Eikelenboom <linux@eikelenboom.it>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Keir Fraser <keir@xen.org>
CC: Jan Beulich <JBeulich@suse.com>
CC: Ian Campbell <Ian.Campbell@citrix.com>
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 xen/common/unlzma.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/common/unlzma.c b/xen/common/unlzma.c
index 103d2df..a7da55b 100644
--- a/xen/common/unlzma.c
+++ b/xen/common/unlzma.c
@@ -214,11 +214,11 @@ rc_bit_tree_decode(struct rc *rc, uint16_t *p, int num_levels, int *symbol)
  */
 
 
-struct __packed lzma_header {
+struct lzma_header {
 	uint8_t pos;
 	uint32_t dict_size;
 	uint64_t dst_size;
-};
+} __attribute__((packed)) ;
 
 
 #define LZMA_BASE_SIZE 1846
-- 
1.7.10.4