I hope that this is the right list for compilation issues.
When building libelf-tools.c with gcc 4.5.4 on NetBSD-current/amd64:
In file included from libelf-private.h:25:0,
from libelf-tools.c:19:
/usr/src/local/xen/xen/include/xen/libelf.h:32:21: fatal error: stdbool.h: No
such file or directory
I ran into this problem when trying to apply XSA-55 to xen 4.2.2, but
just reproduced it in -head.
I think this issue stems from a combination of commit 7a549a6aa
...
libelf: use C99 bool for booleans
...
In this patch we change all the booleans in libelf to C99 bool,
from <stdbool.h>.
and
xen/arch/x86/Rules.mk:
ifneq ($(XEN_OS),SunOS)
CFLAGS-$(gcc) += -nostdinc
endif
If I comment out the -nostdinc in Rules.mk, I get a successful "make
xen".
(One mystery: why aren''t you all seeing this?)
Cheers,
Patrick
>>> On 08.08.13 at 13:49, Patrick Welche <prlw1@cam.ac.uk> wrote: > I hope that this is the right list for compilation issues. > > When building libelf-tools.c with gcc 4.5.4 on NetBSD-current/amd64: > > In file included from libelf-private.h:25:0, > from libelf-tools.c:19: > /usr/src/local/xen/xen/include/xen/libelf.h:32:21: fatal error: stdbool.h: > No such file or directory > > I ran into this problem when trying to apply XSA-55 to xen 4.2.2, but > just reproduced it in -head. > > I think this issue stems from a combination of commit 7a549a6aa > ... > libelf: use C99 bool for booleans > ... > In this patch we change all the booleans in libelf to C99 bool, > from <stdbool.h>. > > and > > xen/arch/x86/Rules.mk: > ifneq ($(XEN_OS),SunOS) > CFLAGS-$(gcc) += -nostdinc > endif > > If I comment out the -nostdinc in Rules.mk, I get a successful "make xen".So perhaps NetBSD then needs a similar override as Solaris. But suppressing -nostdinc is a bad idea in general, and I wonder why this sits in a arch specific makefile instead of in xen/Rules.mk, as this ought to always be in effect for the hypervisor builds.> (One mystery: why aren''t you all seeing this?)No mystery, but also not immediately obvious: -iwithprefix adds the compiler''s include directory to the end of the include search paths, thus allowing stdbool.h and stdarg.h to be found. For stdarg.h (which you ought to have the same problem with in libelf/) xen/stdarg.h already has special treatment for __OpenBSD__ and __NetBSD__ (i.e. avoiding similar problems for all the cases where xen/stdarg.h is used instead of plain stdarg.h). Whether that''s not the case on NetBSD, or whether that directory simply doesn''t exist or is empty you''d need to find out on your installation. Jan
On Thu, Aug 08, 2013 at 02:11:16PM +0100, Jan Beulich wrote:> >>> On 08.08.13 at 13:49, Patrick Welche <prlw1@cam.ac.uk> wrote: > > I hope that this is the right list for compilation issues. > > > > When building libelf-tools.c with gcc 4.5.4 on NetBSD-current/amd64: > > > > In file included from libelf-private.h:25:0, > > from libelf-tools.c:19: > > /usr/src/local/xen/xen/include/xen/libelf.h:32:21: fatal error: stdbool.h: > > No such file or directory > > > > I ran into this problem when trying to apply XSA-55 to xen 4.2.2, but > > just reproduced it in -head. > > > > I think this issue stems from a combination of commit 7a549a6aa > > ... > > libelf: use C99 bool for booleans > > ... > > In this patch we change all the booleans in libelf to C99 bool, > > from <stdbool.h>. > > > > and > > > > xen/arch/x86/Rules.mk: > > ifneq ($(XEN_OS),SunOS) > > CFLAGS-$(gcc) += -nostdinc > > endif > > > > If I comment out the -nostdinc in Rules.mk, I get a successful "make xen". > > So perhaps NetBSD then needs a similar override as Solaris. But > suppressing -nostdinc is a bad idea in general, and I wonder why > this sits in a arch specific makefile instead of in xen/Rules.mk, as > this ought to always be in effect for the hypervisor builds.Indeed: I wondered whether you were all working on the arm port so didn''t see it ;-)> > (One mystery: why aren''t you all seeing this?) > > No mystery, but also not immediately obvious: -iwithprefix adds > the compiler''s include directory to the end of the include search > paths, thus allowing stdbool.h and stdarg.h to be found. For > stdarg.h (which you ought to have the same problem with in > libelf/) xen/stdarg.h already has special treatment for > __OpenBSD__ and __NetBSD__ (i.e. avoiding similar problems > for all the cases where xen/stdarg.h is used instead of plain > stdarg.h). > > Whether that''s not the case on NetBSD, or whether that directory > simply doesn''t exist or is empty you''d need to find out on your > installation.So, in xen/arch/x86/Rules.mk, there is "-iwithprefix include", which means add "include" to the end of the directory defined by the "-iprefix DIR" option. I just looked on an ubuntu 10 box, and gcc -v lists "--prefix=/usr" which seems to be used as the default value of -iprefix. The gcc compiler on the NetBSD box doesn''t list --prefix as one of its configure options, so I don''t know what directory is used as the default prefix. ""? diff --git a/xen/arch/x86/Rules.mk b/xen/arch/x86/Rules.mk index 0a9d68d..223aa1c 100644 --- a/xen/arch/x86/Rules.mk +++ b/xen/arch/x86/Rules.mk @@ -26,7 +26,7 @@ CFLAGS-$(gcc) += -nostdinc endif CFLAGS += -fno-builtin -fno-common -Wredundant-decls -CFLAGS += -iwithprefix include -Werror -Wno-pointer-arith -pipe +CFLAGS += -iprefix /usr/ -iwithprefix include -Werror -Wno-pointer-arith -pipe CFLAGS += -I$(BASEDIR)/include CFLAGS += -I$(BASEDIR)/include/asm-x86/mach-generic CFLAGS += -I$(BASEDIR)/include/asm-x86/mach-default also got me a successful build. (/usr/include/stdbool.h is what we are aiming for.) But is this all worthwhile? We are using the standard header file stdbool.h, telling the preprocessor not to look in the standard system directories with -nostdinc, and then telling the preprocessor, via -iwithprefix, to look in /usr/include, which is the main standard system directory, anyway. Cheers, Patrick
On Thu, 2013-08-08 at 16:18 +0100, Patrick Welche wrote:> On Thu, Aug 08, 2013 at 02:11:16PM +0100, Jan Beulich wrote: > > >>> On 08.08.13 at 13:49, Patrick Welche <prlw1@cam.ac.uk> wrote: > > > I hope that this is the right list for compilation issues. > > > > > > When building libelf-tools.c with gcc 4.5.4 on NetBSD-current/amd64: > > > > > > In file included from libelf-private.h:25:0, > > > from libelf-tools.c:19: > > > /usr/src/local/xen/xen/include/xen/libelf.h:32:21: fatal error: stdbool.h: > > > No such file or directory > > > > > > I ran into this problem when trying to apply XSA-55 to xen 4.2.2, but > > > just reproduced it in -head. > > > > > > I think this issue stems from a combination of commit 7a549a6aa > > > ... > > > libelf: use C99 bool for booleans > > > ... > > > In this patch we change all the booleans in libelf to C99 bool, > > > from <stdbool.h>. > > > > > > and > > > > > > xen/arch/x86/Rules.mk: > > > ifneq ($(XEN_OS),SunOS) > > > CFLAGS-$(gcc) += -nostdinc > > > endif > > > > > > If I comment out the -nostdinc in Rules.mk, I get a successful "make xen". > > > > So perhaps NetBSD then needs a similar override as Solaris. But > > suppressing -nostdinc is a bad idea in general, and I wonder why > > this sits in a arch specific makefile instead of in xen/Rules.mk, as > > this ought to always be in effect for the hypervisor builds. > > Indeed: I wondered whether you were all working on the arm port so didn''t > see it ;-) > > > > (One mystery: why aren''t you all seeing this?) > > > > No mystery, but also not immediately obvious: -iwithprefix adds > > the compiler''s include directory to the end of the include search > > paths, thus allowing stdbool.h and stdarg.h to be found. For > > stdarg.h (which you ought to have the same problem with in > > libelf/) xen/stdarg.h already has special treatment for > > __OpenBSD__ and __NetBSD__ (i.e. avoiding similar problems > > for all the cases where xen/stdarg.h is used instead of plain > > stdarg.h). > > > > Whether that''s not the case on NetBSD, or whether that directory > > simply doesn''t exist or is empty you''d need to find out on your > > installation. > > So, in xen/arch/x86/Rules.mk, there is "-iwithprefix include", > which means add "include" to the end of the directory defined > by the "-iprefix DIR" option. I just looked on an ubuntu 10 box, > and gcc -v lists "--prefix=/usr" which seems to be used as the > default value of -iprefix. The gcc compiler on the NetBSD box > doesn''t list --prefix as one of its configure options, so > I don''t know what directory is used as the default prefix. ""? > > diff --git a/xen/arch/x86/Rules.mk b/xen/arch/x86/Rules.mk > index 0a9d68d..223aa1c 100644 > --- a/xen/arch/x86/Rules.mk > +++ b/xen/arch/x86/Rules.mk > @@ -26,7 +26,7 @@ CFLAGS-$(gcc) += -nostdinc > endif > > CFLAGS += -fno-builtin -fno-common -Wredundant-decls > -CFLAGS += -iwithprefix include -Werror -Wno-pointer-arith -pipe > +CFLAGS += -iprefix /usr/ -iwithprefix include -Werror -Wno-pointer-arith -pipe > CFLAGS += -I$(BASEDIR)/include > CFLAGS += -I$(BASEDIR)/include/asm-x86/mach-generic > CFLAGS += -I$(BASEDIR)/include/asm-x86/mach-default > > also got me a successful build. > (/usr/include/stdbool.h is what we are aiming for.)At least on Debian we are actually aiming for /usr/lib/gcc/x86_64-linux-gnu/X.Y/include/stdbool.h I don''t have /usr/include/stdbool.h. This makes sense since stdbool is, AIUI, a compiler provided interface, not a libc one. Perhaps this is a difference between Linux and BSD? Ian.
>>> On 08.08.13 at 17:18, Patrick Welche <prlw1@cam.ac.uk> wrote: > On Thu, Aug 08, 2013 at 02:11:16PM +0100, Jan Beulich wrote: >> >>> On 08.08.13 at 13:49, Patrick Welche <prlw1@cam.ac.uk> wrote: >> > I hope that this is the right list for compilation issues. >> > >> > When building libelf-tools.c with gcc 4.5.4 on NetBSD-current/amd64: >> > >> > In file included from libelf-private.h:25:0, >> > from libelf-tools.c:19: >> > /usr/src/local/xen/xen/include/xen/libelf.h:32:21: fatal error: stdbool.h: > >> > No such file or directory >> > >> > I ran into this problem when trying to apply XSA-55 to xen 4.2.2, but >> > just reproduced it in -head. >> > >> > I think this issue stems from a combination of commit 7a549a6aa >> > ... >> > libelf: use C99 bool for booleans >> > ... >> > In this patch we change all the booleans in libelf to C99 bool, >> > from <stdbool.h>. >> > >> > and >> > >> > xen/arch/x86/Rules.mk: >> > ifneq ($(XEN_OS),SunOS) >> > CFLAGS-$(gcc) += -nostdinc >> > endif >> > >> > If I comment out the -nostdinc in Rules.mk, I get a successful "make xen". >> >> So perhaps NetBSD then needs a similar override as Solaris. But >> suppressing -nostdinc is a bad idea in general, and I wonder why >> this sits in a arch specific makefile instead of in xen/Rules.mk, as >> this ought to always be in effect for the hypervisor builds. > > Indeed: I wondered whether you were all working on the arm port so didn''t > see it ;-) > >> > (One mystery: why aren''t you all seeing this?) >> >> No mystery, but also not immediately obvious: -iwithprefix adds >> the compiler''s include directory to the end of the include search >> paths, thus allowing stdbool.h and stdarg.h to be found. For >> stdarg.h (which you ought to have the same problem with in >> libelf/) xen/stdarg.h already has special treatment for >> __OpenBSD__ and __NetBSD__ (i.e. avoiding similar problems >> for all the cases where xen/stdarg.h is used instead of plain >> stdarg.h). >> >> Whether that''s not the case on NetBSD, or whether that directory >> simply doesn''t exist or is empty you''d need to find out on your >> installation. > > So, in xen/arch/x86/Rules.mk, there is "-iwithprefix include", > which means add "include" to the end of the directory defined > by the "-iprefix DIR" option. I just looked on an ubuntu 10 box, > and gcc -v lists "--prefix=/usr" which seems to be used as the > default value of -iprefix. The gcc compiler on the NetBSD box > doesn''t list --prefix as one of its configure options, so > I don''t know what directory is used as the default prefix. ""?No, according to my checking, the --prefix configure option listed does not correlate with the directory where the header is found.> --- a/xen/arch/x86/Rules.mk > +++ b/xen/arch/x86/Rules.mk > @@ -26,7 +26,7 @@ CFLAGS-$(gcc) += -nostdinc > endif > > CFLAGS += -fno-builtin -fno-common -Wredundant-decls > -CFLAGS += -iwithprefix include -Werror -Wno-pointer-arith -pipe > +CFLAGS += -iprefix /usr/ -iwithprefix include -Werror -Wno-pointer-arith -pipe > CFLAGS += -I$(BASEDIR)/include > CFLAGS += -I$(BASEDIR)/include/asm-x86/mach-generic > CFLAGS += -I$(BASEDIR)/include/asm-x86/mach-default > > also got me a successful build. > (/usr/include/stdbool.h is what we are aiming for.) > > But is this all worthwhile? We are using the standard header file > stdbool.h, telling the preprocessor not to look in the standard > system directories with -nostdinc, and then telling the preprocessor, > via -iwithprefix, to look in /usr/include, which is the main standard > system directory, anyway.No, just go check adding -v to the compiler options. For me, the directory is underneath where the gcc binaries (cc1 et al) are. And we definitely don''t want to have /usr/include or some such in our include path. Jan
On Thu, Aug 08, 2013 at 04:23:57PM +0100, Ian Campbell wrote:> On Thu, 2013-08-08 at 16:18 +0100, Patrick Welche wrote: > > On Thu, Aug 08, 2013 at 02:11:16PM +0100, Jan Beulich wrote: > > > >>> On 08.08.13 at 13:49, Patrick Welche <prlw1@cam.ac.uk> wrote: > > > > I hope that this is the right list for compilation issues. > > > > > > > > When building libelf-tools.c with gcc 4.5.4 on NetBSD-current/amd64: > > > > > > > > In file included from libelf-private.h:25:0, > > > > from libelf-tools.c:19: > > > > /usr/src/local/xen/xen/include/xen/libelf.h:32:21: fatal error: stdbool.h: > > > > No such file or directory[...]> > > No mystery, but also not immediately obvious: -iwithprefix adds > > > the compiler''s include directory to the end of the include search > > > paths, thus allowing stdbool.h and stdarg.h to be found. For > > > stdarg.h (which you ought to have the same problem with in > > > libelf/) xen/stdarg.h already has special treatment for > > > __OpenBSD__ and __NetBSD__ (i.e. avoiding similar problems > > > for all the cases where xen/stdarg.h is used instead of plain > > > stdarg.h). > > > > > > Whether that''s not the case on NetBSD, or whether that directory > > > simply doesn''t exist or is empty you''d need to find out on your > > > installation. > > > > So, in xen/arch/x86/Rules.mk, there is "-iwithprefix include", > > which means add "include" to the end of the directory defined > > by the "-iprefix DIR" option. I just looked on an ubuntu 10 box, > > and gcc -v lists "--prefix=/usr" which seems to be used as the > > default value of -iprefix. The gcc compiler on the NetBSD box > > doesn''t list --prefix as one of its configure options, so > > I don''t know what directory is used as the default prefix. ""?After Ian''s comment below - that bit of "--prefix=/usr" theory is wrong as on the ubuntu box stdbool.h is indeed under /usr/lib/gcc... and the default prefix is where gcc "staging files" are kept. [...]> > (/usr/include/stdbool.h is what we are aiming for.) > > At least on Debian we are actually aiming > for /usr/lib/gcc/x86_64-linux-gnu/X.Y/include/stdbool.h > > I don''t have /usr/include/stdbool.h. This makes sense since stdbool is, > AIUI, a compiler provided interface, not a libc one. > > Perhaps this is a difference between Linux and BSD?So it seems. According to stdbool(3) on the NetBSD box STANDARDS The <stdbool.h> header conforms to ISO/IEC 9899:1999 (``ISO C99'''') and IEEE Std 1003.1-2001 (``POSIX.1''''). HISTORY The <stdbool.h> header was first introduced in NetBSD 1.6. and ominously The ability to undefine and redefine the macros bool, true, and false is an obsolescent feature that may be withdrawn in a future version of a standard. Does this have implications for the orginal XSA-55? Cheers, Patrick
On Thu, Aug 08, 2013 at 04:30:06PM +0100, Jan Beulich wrote:> No, according to my checking, the --prefix configure option > listed does not correlate with the directory where the header > is found.Yes - I think our emails crossed... The underlying problem is: Linux: /usr/lib/gcc/i?86-linux-gnu/n.m/include/stdbool.h NetBSD: /usr/include/stdbool.h Cheers, Patrick
(adding Ian J who did most of XSA-55) On Thu, 2013-08-08 at 16:47 +0100, Patrick Welche wrote:> On Thu, Aug 08, 2013 at 04:30:06PM +0100, Jan Beulich wrote: > > No, according to my checking, the --prefix configure option > > listed does not correlate with the directory where the header > > is found. > > Yes - I think our emails crossed... > > The underlying problem is: > > Linux: /usr/lib/gcc/i?86-linux-gnu/n.m/include/stdbool.h > NetBSD: /usr/include/stdbool.hThe hypervisor side, which is where --nostdinc is used, has it''s own bool_t in asm/types.h. Perhaps libelf (which is supposed to compile for both user and hypervisor space) should be using #ifdef __XEN__ #include <asm/types.h> #else #include <stdbool.h> #endif ? I''d be a bit concerned about the fact that Xen''s bool_t is a typedef for char, as opposed to the compilers typedef to _Bool which has special meaning. It may not matter in practice but might the fact that _Bool canonicalises to 0 or 1 vs. 0 or !0 cause something subtle? ian.
On Thu, Aug 08, 2013 at 05:12:51PM +0100, Ian Campbell wrote:> (adding Ian J who did most of XSA-55) > On Thu, 2013-08-08 at 16:47 +0100, Patrick Welche wrote: > > On Thu, Aug 08, 2013 at 04:30:06PM +0100, Jan Beulich wrote: > > > No, according to my checking, the --prefix configure option > > > listed does not correlate with the directory where the header > > > is found. > > > > Yes - I think our emails crossed... > > > > The underlying problem is: > > > > Linux: /usr/lib/gcc/i?86-linux-gnu/n.m/include/stdbool.h > > NetBSD: /usr/include/stdbool.h > > The hypervisor side, which is where --nostdinc is used, has it''s own > bool_t in asm/types.h. Perhaps libelf (which is supposed to compile for > both user and hypervisor space) should be using > #ifdef __XEN__ > #include <asm/types.h> > #else > #include <stdbool.h> > #endif > > ? > > I''d be a bit concerned about the fact that Xen''s bool_t is a typedef for > char, as opposed to the compilers typedef to _Bool which has special > meaning. It may not matter in practice but might the fact that _Bool > canonicalises to 0 or 1 vs. 0 or !0 cause something subtle?AFAIK char c=0; !c == 1 (true) (rather than 0xff or ~c or whatever - but this is without a "malicious compiler") so at a glance, this seems OK. (But then I don''t know the original motivation for replacing bools in XSA-55...) Cheers, Patrick
On 08/08/2013 18:26, Patrick Welche wrote:> On Thu, Aug 08, 2013 at 05:12:51PM +0100, Ian Campbell wrote: >> (adding Ian J who did most of XSA-55) >> On Thu, 2013-08-08 at 16:47 +0100, Patrick Welche wrote: >>> On Thu, Aug 08, 2013 at 04:30:06PM +0100, Jan Beulich wrote: >>>> No, according to my checking, the --prefix configure option >>>> listed does not correlate with the directory where the header >>>> is found. >>> Yes - I think our emails crossed... >>> >>> The underlying problem is: >>> >>> Linux: /usr/lib/gcc/i?86-linux-gnu/n.m/include/stdbool.h >>> NetBSD: /usr/include/stdbool.h >> The hypervisor side, which is where --nostdinc is used, has it''s own >> bool_t in asm/types.h. Perhaps libelf (which is supposed to compile for >> both user and hypervisor space) should be using >> #ifdef __XEN__ >> #include <asm/types.h> >> #else >> #include <stdbool.h> >> #endif >> >> ? >> >> I''d be a bit concerned about the fact that Xen''s bool_t is a typedef for >> char, as opposed to the compilers typedef to _Bool which has special >> meaning. It may not matter in practice but might the fact that _Bool >> canonicalises to 0 or 1 vs. 0 or !0 cause something subtle? > AFAIK > > char c=0; > !c == 1 (true) (rather than 0xff or ~c or whatever - but this is without > a "malicious compiler") > > so at a glance, this seems OK. (But then I don''t know the original > motivation for replacing bools in XSA-55...) > > Cheers, > > PatrickXSA-55 ended up fighting against the C specification. Under C, the act of creating an invalid pointer is itself undefined. Therefore, taking a base address and adding an offset is possibly undefined, depending on whether the offset lives within the malloc()''d space or not. As a result, the range check can be optimised away, because if it can be proved to be correct, it will always pass, and if it is isn''t the undefined behaviour rules can allow it to also be true. This means that an aggressive optimising compiler can (and, I am informed, does) optimise away range checks, resulting in code which reads as secure, but compiles as insecure. The changes for XSA-55 resulted in exercising as many guarantees from the C standard as much, and working around the rest. As you might notice from some of the larger patches, structure access (on untrusted structures) is reimplemented as macros and unions, so as to be guaranteed to not be optimised away, even by the most aggressive compilers. ~Andrew> > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
On Thu, 2013-08-08 at 20:05 +0100, Andrew Cooper wrote:> On 08/08/2013 18:26, Patrick Welche wrote: > > On Thu, Aug 08, 2013 at 05:12:51PM +0100, Ian Campbell wrote: > >> (adding Ian J who did most of XSA-55) > >> On Thu, 2013-08-08 at 16:47 +0100, Patrick Welche wrote: > >>> On Thu, Aug 08, 2013 at 04:30:06PM +0100, Jan Beulich wrote: > >>>> No, according to my checking, the --prefix configure option > >>>> listed does not correlate with the directory where the header > >>>> is found. > >>> Yes - I think our emails crossed... > >>> > >>> The underlying problem is: > >>> > >>> Linux: /usr/lib/gcc/i?86-linux-gnu/n.m/include/stdbool.h > >>> NetBSD: /usr/include/stdbool.h > >> The hypervisor side, which is where --nostdinc is used, has it''s own > >> bool_t in asm/types.h. Perhaps libelf (which is supposed to compile for > >> both user and hypervisor space) should be using > >> #ifdef __XEN__ > >> #include <asm/types.h> > >> #else > >> #include <stdbool.h> > >> #endif > >> > >> ? > >> > >> I''d be a bit concerned about the fact that Xen''s bool_t is a typedef for > >> char, as opposed to the compilers typedef to _Bool which has special > >> meaning. It may not matter in practice but might the fact that _Bool > >> canonicalises to 0 or 1 vs. 0 or !0 cause something subtle? > > AFAIK > > > > char c=0; > > !c == 1 (true) (rather than 0xff or ~c or whatever - but this is without > > a "malicious compiler") > > > > so at a glance, this seems OK. (But then I don''t know the original > > motivation for replacing bools in XSA-55...) > > > > Cheers, > > > > Patrick > > XSA-55 ended up fighting against the C specification. > > Under C, the act of creating an invalid pointer is itself undefined. > Therefore, taking a base address and adding an offset is possibly > undefined, depending on whether the offset lives within the malloc()''d > space or not. As a result, the range check can be optimised away, > because if it can be proved to be correct, it will always pass, and if > it is isn''t the undefined behaviour rules can allow it to also be true. > > This means that an aggressive optimising compiler can (and, I am > informed, does) optimise away range checks, resulting in code which > reads as secure, but compiles as insecure. > > The changes for XSA-55 resulted in exercising as many guarantees from > the C standard as much, and working around the rest. As you might > notice from some of the larger patches, structure access (on untrusted > structures) is reimplemented as macros and unions, so as to be > guaranteed to not be optimised away, even by the most aggressive compilers.What does any of that have to do with the use of stdbool? Ian.
On 08/08/2013 20:24, Ian Campbell wrote:> On Thu, 2013-08-08 at 20:05 +0100, Andrew Cooper wrote: >> On 08/08/2013 18:26, Patrick Welche wrote: >>> On Thu, Aug 08, 2013 at 05:12:51PM +0100, Ian Campbell wrote: >>>> (adding Ian J who did most of XSA-55) >>>> On Thu, 2013-08-08 at 16:47 +0100, Patrick Welche wrote: >>>>> On Thu, Aug 08, 2013 at 04:30:06PM +0100, Jan Beulich wrote: >>>>>> No, according to my checking, the --prefix configure option >>>>>> listed does not correlate with the directory where the header >>>>>> is found. >>>>> Yes - I think our emails crossed... >>>>> >>>>> The underlying problem is: >>>>> >>>>> Linux: /usr/lib/gcc/i?86-linux-gnu/n.m/include/stdbool.h >>>>> NetBSD: /usr/include/stdbool.h >>>> The hypervisor side, which is where --nostdinc is used, has it''s own >>>> bool_t in asm/types.h. Perhaps libelf (which is supposed to compile for >>>> both user and hypervisor space) should be using >>>> #ifdef __XEN__ >>>> #include <asm/types.h> >>>> #else >>>> #include <stdbool.h> >>>> #endif >>>> >>>> ? >>>> >>>> I''d be a bit concerned about the fact that Xen''s bool_t is a typedef for >>>> char, as opposed to the compilers typedef to _Bool which has special >>>> meaning. It may not matter in practice but might the fact that _Bool >>>> canonicalises to 0 or 1 vs. 0 or !0 cause something subtle? >>> AFAIK >>> >>> char c=0; >>> !c == 1 (true) (rather than 0xff or ~c or whatever - but this is without >>> a "malicious compiler") >>> >>> so at a glance, this seems OK. (But then I don''t know the original >>> motivation for replacing bools in XSA-55...) >>> >>> Cheers, >>> >>> Patrick >> XSA-55 ended up fighting against the C specification. >> >> Under C, the act of creating an invalid pointer is itself undefined. >> Therefore, taking a base address and adding an offset is possibly >> undefined, depending on whether the offset lives within the malloc()''d >> space or not. As a result, the range check can be optimised away, >> because if it can be proved to be correct, it will always pass, and if >> it is isn''t the undefined behaviour rules can allow it to also be true. >> >> This means that an aggressive optimising compiler can (and, I am >> informed, does) optimise away range checks, resulting in code which >> reads as secure, but compiles as insecure. >> >> The changes for XSA-55 resulted in exercising as many guarantees from >> the C standard as much, and working around the rest. As you might >> notice from some of the larger patches, structure access (on untrusted >> structures) is reimplemented as macros and unions, so as to be >> guaranteed to not be optimised away, even by the most aggressive compilers. > What does any of that have to do with the use of stdbool? > > Ian. > >Sorry - I guess I didn''t make that as clear as I was intending to. "exercising as many guarantees from the C standard" Part of the proof that the new code was good involved turning the over-use of ints to other types. Some to unsigned integers (for array indicies). Turning the ints used as booleans to _Bools helped reduce the noise. IIRC, there might have been a place where ints (expecting to be bools) were added to a base, which was functionally broken if the int contained anything other than 0 or 1. ~Andrew
>>> On 08.08.13 at 17:47, Patrick Welche <prlw1@cam.ac.uk> wrote: > On Thu, Aug 08, 2013 at 04:30:06PM +0100, Jan Beulich wrote: >> No, according to my checking, the --prefix configure option >> listed does not correlate with the directory where the header >> is found. > > Yes - I think our emails crossed... > > The underlying problem is: > > Linux: /usr/lib/gcc/i?86-linux-gnu/n.m/include/stdbool.h > NetBSD: /usr/include/stdbool.hSo then my original recommendation holds: Extend the Solaris workaround to NetBSD, but at the same time generalize it by moving it from xen/arch/*/Rules.mk to xen/Rules.mk. Jan
>>> On 08.08.13 at 18:12, Ian Campbell <Ian.Campbell@citrix.com> wrote: > (adding Ian J who did most of XSA-55) > On Thu, 2013-08-08 at 16:47 +0100, Patrick Welche wrote: >> On Thu, Aug 08, 2013 at 04:30:06PM +0100, Jan Beulich wrote: >> > No, according to my checking, the --prefix configure option >> > listed does not correlate with the directory where the header >> > is found. >> >> Yes - I think our emails crossed... >> >> The underlying problem is: >> >> Linux: /usr/lib/gcc/i?86-linux-gnu/n.m/include/stdbool.h >> NetBSD: /usr/include/stdbool.h > > The hypervisor side, which is where --nostdinc is used, has it''s own > bool_t in asm/types.h. Perhaps libelf (which is supposed to compile for > both user and hypervisor space) should be using > #ifdef __XEN__ > #include <asm/types.h> > #else > #include <stdbool.h> > #endif > > ?That would make sense only if we could also do the same for stdarg.h, but you''ll note that xen/stdarg.h already works around the same problem on NetBSD and OpenBSD. Going through the history of xen/stdarg.h also shows that this has been a recurring problem. It escapes me why they can''t just play things the gcc way if gcc is their compiler. I.e. either we allow ourselves to use standard headers that are defining only compiler determined things (in which case we could also use stdint.h for example), or we don''t use _any_ standard headers.> I''d be a bit concerned about the fact that Xen''s bool_t is a typedef for > char, as opposed to the compilers typedef to _Bool which has special > meaning. It may not matter in practice but might the fact that _Bool > canonicalises to 0 or 1 vs. 0 or !0 cause something subtle?No, that won''t work without auditing the code: Non-boolean expression results will be converted to 0/1 by the compiler when the lvalue''s type is _Bool, but won''t when it''s bool_t. While that might seem fine at the first glance as long as consumers of such variables don''t do explicit == 1 checks, this is becoming a problem when the non-boolean result is 0 modulo 256 (since the conversion done in the non-_Bool case is a truncation). Jan
On Fri, Aug 09, 2013 at 07:44:26AM +0100, Jan Beulich wrote:> >>> On 08.08.13 at 17:47, Patrick Welche <prlw1@cam.ac.uk> wrote: > > On Thu, Aug 08, 2013 at 04:30:06PM +0100, Jan Beulich wrote: > >> No, according to my checking, the --prefix configure option > >> listed does not correlate with the directory where the header > >> is found. > > > > Yes - I think our emails crossed... > > > > The underlying problem is: > > > > Linux: /usr/lib/gcc/i?86-linux-gnu/n.m/include/stdbool.h > > NetBSD: /usr/include/stdbool.h > > So then my original recommendation holds: Extend the Solaris > workaround to NetBSD, but at the same time generalize it by > moving it from xen/arch/*/Rules.mk to xen/Rules.mk.Ian C''s asm/types.h suggestion also gives me a more or less successful build - at least all libelf parts are fine. (-head fails on tools with libxlu_cfg_y.c: In function ''xlu__cfg_yyparse'': libxlu_cfg_y.c:1292:41: error: ''scanner'' undeclared (first use in this function) but the repository files were automatically regenerated e.g. with bison 3, and flex may have been "tweaked") Cheers, Patrick
On Fri, Aug 09, 2013 at 08:50:32AM +0100, Jan Beulich wrote:> That would make sense only if we could also do the same for > stdarg.h, but you''ll note that xen/stdarg.h already works around > the same problem on NetBSD and OpenBSD. Going through the > history of xen/stdarg.h also shows that this has been a recurring > problem. It escapes me why they can''t just play things the gcc > way if gcc is their compiler.The plan is to use llvm/clang - I haven''t tried it, though others already use it as their default compiler (the OS certainly builds).> I.e. either we allow ourselves to use standard headers that are > defining only compiler determined things (in which case we could > also use stdint.h for example), or we don''t use _any_ standard > headers. > > > I''d be a bit concerned about the fact that Xen''s bool_t is a typedef for > > char, as opposed to the compilers typedef to _Bool which has special > > meaning. It may not matter in practice but might the fact that _Bool > > canonicalises to 0 or 1 vs. 0 or !0 cause something subtle? > > No, that won''t work without auditing the code: Non-boolean > expression results will be converted to 0/1 by the compiler when > the lvalue''s type is _Bool, but won''t when it''s bool_t. While that > might seem fine at the first glance as long as consumers of such > variables don''t do explicit == 1 checks, this is becoming a problem > when the non-boolean result is 0 modulo 256 (since the conversion > done in the non-_Bool case is a truncation).I''m sorry, I still don''t see how I can write code which exhibits the problem case... Could I have a 1A supervision please? Cheers, Patrick
>>> On 09.08.13 at 10:11, Patrick Welche <prlw1@cam.ac.uk> wrote: > I''m sorry, I still don''t see how I can write code which exhibits the > problem case... Could I have a 1A supervision please?Try this #include <stdbool.h> #include <stdio.h> #include <stdlib.h> typedef char bool_t; int main(int argc, char*argv[]) { while(*++argv) { long l = strtol(*argv, NULL, 0); bool b1 = l; bool_t b2 = l; printf("%ld -> %d/%d\n", l, b1, b2); } return 0; } with e.g. inputs 1, 255, and 256. Jan
On Fri, Aug 09, 2013 at 09:16:52AM +0100, Jan Beulich wrote:> >>> On 09.08.13 at 10:11, Patrick Welche <prlw1@cam.ac.uk> wrote: > > I''m sorry, I still don''t see how I can write code which exhibits the > > problem case... Could I have a 1A supervision please?> with e.g. inputs 1, 255, and 256.I see - if you start with 255 or 256, not if use 0,1 and combinations of ! == etc. I didn''t use - or ~ as I didn''t see any in the libelf bits. Still, point taken, thanks, Patrick
On Fri, Aug 09, 2013 at 09:11:25AM +0100, Patrick Welche wrote:> On Fri, Aug 09, 2013 at 08:50:32AM +0100, Jan Beulich wrote: > > That would make sense only if we could also do the same for > > stdarg.h, but you''ll note that xen/stdarg.h already works around > > the same problem on NetBSD and OpenBSD. Going through the > > history of xen/stdarg.h also shows that this has been a recurring > > problem. It escapes me why they can''t just play things the gcc > > way if gcc is their compiler. > > The plan is to use llvm/clang - I haven''t tried it, though others > already use it as their default compiler (the OS certainly builds).This part seems to already be answered in xen/arch/x86/Rules.mk: # Solaris grabs stdarg.h and friends from the system include directory. # Clang likewise. -> compiler rather than OS check? Cheers, Patrick
>>> On 09.08.13 at 10:33, Patrick Welche <prlw1@cam.ac.uk> wrote: > On Fri, Aug 09, 2013 at 09:11:25AM +0100, Patrick Welche wrote: >> On Fri, Aug 09, 2013 at 08:50:32AM +0100, Jan Beulich wrote: >> > That would make sense only if we could also do the same for >> > stdarg.h, but you''ll note that xen/stdarg.h already works around >> > the same problem on NetBSD and OpenBSD. Going through the >> > history of xen/stdarg.h also shows that this has been a recurring >> > problem. It escapes me why they can''t just play things the gcc >> > way if gcc is their compiler. >> >> The plan is to use llvm/clang - I haven''t tried it, though others >> already use it as their default compiler (the OS certainly builds). > > This part seems to already be answered in xen/arch/x86/Rules.mk: > > # Solaris grabs stdarg.h and friends from the system include directory. > # Clang likewise. > > -> compiler rather than OS check?Both, as is already done there. Albeit I don''t know clang at all, in particular whether it - like gcc - makes its own header versions available somewhere. Jan
At 09:40 +0100 on 09 Aug (1376041248), Jan Beulich wrote:> >>> On 09.08.13 at 10:33, Patrick Welche <prlw1@cam.ac.uk> wrote: > > On Fri, Aug 09, 2013 at 09:11:25AM +0100, Patrick Welche wrote: > >> On Fri, Aug 09, 2013 at 08:50:32AM +0100, Jan Beulich wrote: > >> > That would make sense only if we could also do the same for > >> > stdarg.h, but you''ll note that xen/stdarg.h already works around > >> > the same problem on NetBSD and OpenBSD. Going through the > >> > history of xen/stdarg.h also shows that this has been a recurring > >> > problem. It escapes me why they can''t just play things the gcc > >> > way if gcc is their compiler. > >> > >> The plan is to use llvm/clang - I haven''t tried it, though others > >> already use it as their default compiler (the OS certainly builds). > > > > This part seems to already be answered in xen/arch/x86/Rules.mk: > > > > # Solaris grabs stdarg.h and friends from the system include directory. > > # Clang likewise. > > > > -> compiler rather than OS check? > > Both, as is already done there. Albeit I don''t know clang at all, > in particular whether it - like gcc - makes its own header versions > available somewhere.That rune is wrong for clang, AFAICT (even though I bet I wrote it). On my local debian/linux system, clang gets stdarg, stdbool &c from /usr/include/clang/3.0/include/ Figuring out how to make clang tell me that path in a scriptable way might be interesting though. I''ll look at it next week. Tim.
On Fri, Aug 09, 2013 at 04:13:29PM +0100, Tim Deegan wrote:> At 09:40 +0100 on 09 Aug (1376041248), Jan Beulich wrote: > > >>> On 09.08.13 at 10:33, Patrick Welche <prlw1@cam.ac.uk> wrote: > > > On Fri, Aug 09, 2013 at 09:11:25AM +0100, Patrick Welche wrote: > > >> On Fri, Aug 09, 2013 at 08:50:32AM +0100, Jan Beulich wrote: > > >> > That would make sense only if we could also do the same for > > >> > stdarg.h, but you''ll note that xen/stdarg.h already works around > > >> > the same problem on NetBSD and OpenBSD. Going through the > > >> > history of xen/stdarg.h also shows that this has been a recurring > > >> > problem. It escapes me why they can''t just play things the gcc > > >> > way if gcc is their compiler. > > >> > > >> The plan is to use llvm/clang - I haven''t tried it, though others > > >> already use it as their default compiler (the OS certainly builds). > > > > > > This part seems to already be answered in xen/arch/x86/Rules.mk: > > > > > > # Solaris grabs stdarg.h and friends from the system include directory. > > > # Clang likewise. > > > > > > -> compiler rather than OS check? > > > > Both, as is already done there. Albeit I don''t know clang at all, > > in particular whether it - like gcc - makes its own header versions > > available somewhere. > > That rune is wrong for clang, AFAICT (even though I bet I wrote it). > On my local debian/linux system, clang gets stdarg, stdbool &c from > /usr/include/clang/3.0/include/ > > Figuring out how to make clang tell me that path in a scriptable way > might be interesting though. I''ll look at it next week.And just for consistency, a clang on NetBSD installation has it in /usr/include... Cheers, Patrick
On Fri, Aug 09, 2013 at 07:44:26AM +0100, Jan Beulich wrote:> >>> On 08.08.13 at 17:47, Patrick Welche <prlw1@cam.ac.uk> wrote: > > On Thu, Aug 08, 2013 at 04:30:06PM +0100, Jan Beulich wrote: > >> No, according to my checking, the --prefix configure option > >> listed does not correlate with the directory where the header > >> is found. > > > > Yes - I think our emails crossed... > > > > The underlying problem is: > > > > Linux: /usr/lib/gcc/i?86-linux-gnu/n.m/include/stdbool.h > > NetBSD: /usr/include/stdbool.h > > So then my original recommendation holds: Extend the Solaris > workaround to NetBSD, but at the same time generalize it by > moving it from xen/arch/*/Rules.mk to xen/Rules.mk.So something like the attached would do the trick... (I had a successful "make xen" with this on -head) Cheers, Patrick _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
>>> On 11.08.13 at 18:41, Patrick Welche <prlw1@cam.ac.uk> wrote: > On Fri, Aug 09, 2013 at 07:44:26AM +0100, Jan Beulich wrote: >> >>> On 08.08.13 at 17:47, Patrick Welche <prlw1@cam.ac.uk> wrote: >> > On Thu, Aug 08, 2013 at 04:30:06PM +0100, Jan Beulich wrote: >> >> No, according to my checking, the --prefix configure option >> >> listed does not correlate with the directory where the header >> >> is found. >> > >> > Yes - I think our emails crossed... >> > >> > The underlying problem is: >> > >> > Linux: /usr/lib/gcc/i?86-linux-gnu/n.m/include/stdbool.h >> > NetBSD: /usr/include/stdbool.h >> >> So then my original recommendation holds: Extend the Solaris >> workaround to NetBSD, but at the same time generalize it by >> moving it from xen/arch/*/Rules.mk to xen/Rules.mk. > > So something like the attached would do the trick... > (I had a successful "make xen" with this on -head)Yes, something like this.>+ifeq ($(XEN_OS),SunOS) >+else ifeq ($(XEN_OS),NetBSD) >+else >+CFLAGS-$(gcc) += -nostdinc >+endifThis may be better expressed with $(filter-out ...).>--- a/xen/arch/x86/Rules.mk >+++ b/xen/arch/x86/Rules.mkThe "-iwithprefix include" should likely be moved to the common Rules.mk too (and then also be removed from the ARM variant), as it is tightly connected with the -nostdinc. Jan
On 08.08.13 17:23, Ian Campbell wrote:> On Thu, 2013-08-08 at 16:18 +0100, Patrick Welche wrote: >> On Thu, Aug 08, 2013 at 02:11:16PM +0100, Jan Beulich wrote: >>>>>> On 08.08.13 at 13:49, Patrick Welche <prlw1@cam.ac.uk> wrote: >>>> I hope that this is the right list for compilation issues. >>>> >>>> When building libelf-tools.c with gcc 4.5.4 on NetBSD-current/amd64: >>>> >>>> In file included from libelf-private.h:25:0, >>>> from libelf-tools.c:19: >>>> /usr/src/local/xen/xen/include/xen/libelf.h:32:21: fatal error: stdbool.h: >>>> No such file or directory >>>> >>>> I ran into this problem when trying to apply XSA-55 to xen 4.2.2, but >>>> just reproduced it in -head. >>>> >>>> I think this issue stems from a combination of commit 7a549a6aa >>>> ... >>>> libelf: use C99 bool for booleans >>>> ... >>>> In this patch we change all the booleans in libelf to C99 bool, >>>> from <stdbool.h>. >>>> >>>> and >>>> >>>> xen/arch/x86/Rules.mk: >>>> ifneq ($(XEN_OS),SunOS) >>>> CFLAGS-$(gcc) += -nostdinc >>>> endif >>>> >>>> If I comment out the -nostdinc in Rules.mk, I get a successful "make xen". >>> >>> So perhaps NetBSD then needs a similar override as Solaris. But >>> suppressing -nostdinc is a bad idea in general, and I wonder why >>> this sits in a arch specific makefile instead of in xen/Rules.mk, as >>> this ought to always be in effect for the hypervisor builds. >> >> Indeed: I wondered whether you were all working on the arm port so didn''t >> see it ;-) >> >>>> (One mystery: why aren''t you all seeing this?) >>> >>> No mystery, but also not immediately obvious: -iwithprefix adds >>> the compiler''s include directory to the end of the include search >>> paths, thus allowing stdbool.h and stdarg.h to be found. For >>> stdarg.h (which you ought to have the same problem with in >>> libelf/) xen/stdarg.h already has special treatment for >>> __OpenBSD__ and __NetBSD__ (i.e. avoiding similar problems >>> for all the cases where xen/stdarg.h is used instead of plain >>> stdarg.h). >>> >>> Whether that''s not the case on NetBSD, or whether that directory >>> simply doesn''t exist or is empty you''d need to find out on your >>> installation. >> >> So, in xen/arch/x86/Rules.mk, there is "-iwithprefix include", >> which means add "include" to the end of the directory defined >> by the "-iprefix DIR" option. I just looked on an ubuntu 10 box, >> and gcc -v lists "--prefix=/usr" which seems to be used as the >> default value of -iprefix. The gcc compiler on the NetBSD box >> doesn''t list --prefix as one of its configure options, so >> I don''t know what directory is used as the default prefix. ""? >> >> diff --git a/xen/arch/x86/Rules.mk b/xen/arch/x86/Rules.mk >> index 0a9d68d..223aa1c 100644 >> --- a/xen/arch/x86/Rules.mk >> +++ b/xen/arch/x86/Rules.mk >> @@ -26,7 +26,7 @@ CFLAGS-$(gcc) += -nostdinc >> endif >> >> CFLAGS += -fno-builtin -fno-common -Wredundant-decls >> -CFLAGS += -iwithprefix include -Werror -Wno-pointer-arith -pipe >> +CFLAGS += -iprefix /usr/ -iwithprefix include -Werror -Wno-pointer-arith -pipe >> CFLAGS += -I$(BASEDIR)/include >> CFLAGS += -I$(BASEDIR)/include/asm-x86/mach-generic >> CFLAGS += -I$(BASEDIR)/include/asm-x86/mach-default >> >> also got me a successful build. >> (/usr/include/stdbool.h is what we are aiming for.) > > At least on Debian we are actually aiming > for /usr/lib/gcc/x86_64-linux-gnu/X.Y/include/stdbool.h > > I don''t have /usr/include/stdbool.h. This makes sense since stdbool is, > AIUI, a compiler provided interface, not a libc one. > > Perhaps this is a difference between Linux and BSD?NetBSD does not use the compiler provided interface and this is why gcc -print-search-dirs | grep install is empty and stubdom does not build. Adding Joerg Sonnenberger to CC. He can explain the details why NetBSD does not use compiler provided interfaces. Christoph