From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: stdbool.h -nostdinc XSA-55 trouble Date: Thu, 8 Aug 2013 20:52:24 +0100 Message-ID: <5203F6F8.4070903@citrix.com> References: <20130808114945.GG22372@quark> <5203B51402000078000EA534@nat28.tlf.novell.com> <20130808151835.GB870@quark> <5203D59E02000078000EA634@nat28.tlf.novell.com> <20130808154714.GD870@quark> <1375978371.14651.12.camel@kazak.uk.xensource.com> <20130808172655.GH870@quark> <5203EBDD.7090007@citrix.com> <1375989850.15681.95.camel@dagon.hellion.org.uk> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1375989850.15681.95.camel@dagon.hellion.org.uk> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: Patrick Welche , Ian Jackson , Jan Beulich , xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org 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 >>>> #else >>>> #include >>>> #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