[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] NPTL/TLS segment flipping code problem
>> Looking at this code (2.0.2), it appears to have a couple of problems I >> could not find mentioned on the mailing list archive: >> >> (1) If base is zero in an expand-up segment, the conversion will yield >> an expand-down segment covering the whole 4Gb, thus providing a >> mechanism to obtain access to XEN space. > >No it won't. When we flip to expands-down we set the limit to > (-(base & PAGE_MASK) >> 12) - 1 > == (-(0 & PAGE_MASK) >> 12) - 1 > == 0 - 1 > == 0xfffff > >This is a *zero-length* grows-down segment, exactly as we require for >safety. [Yes, you can have a zero-length grows-down segment, even >though it is impossible to have a zero-length grows-up segment -- I >tested on real silicon.] Oh, right, I didn't connect the -1 done after the flip: label. But it seems pointless to flip a segment with a base of zero; the gp fault must have had a different reason (i.e. by flipping it to a zero-length segment you'll only cause another gp fault right away when restarting the instruction). >> (2) If a malicious program accesses memory at a small negative offset >> from gs:0 and the access extends into the positive range, the access >> will gp-fault with either descriptor setting, thus leading to an endless >> loop of flipping between the two states. > >Harmless. The guest OS will still execute (e.g., to service timer >interrupts) and will be able to preempt the malicious program when it >has received its timeslice. So the program cannot take over the >machine -- it can only receive the same amount of CPU as a user-space >infinite CPU loop. Not exactly: if a user mode process runs en infinite loop, it'll not block out interrupts for any more than a single instruction. For the mentioned scenario, (physical) interrupt latency would significantly increase. >> (3) Since escaped opcodes (those starting with 0F) aren't handled, >> accessing mm/xmm data in __thread variables (along with other >> specialized operations on such variable the compiler might generate) is >> going to kill the program. Of course, it is similarly problematic that >> SIB addressing still isn't implemented, but that's at least stated so in >> the code. > >Yes there are restrictions in what it supports. But we have a DPRINTK >for those cases so we can fix them up as/if they occur. Except that the DPRINTK expands to nothing in the published sources, so one would never know what caused a spurious SEGV seen in a client OSes app without re-running the whole thing with a version of XEN where these DPRINTKs are actually doing something. >> (4) In the no-mod-r/m handling of the decoder, the byte case is handled >> incorrectly: The address it deals with is still a 32-byte (or 16-byte, >> but 16-bit addressing isn't handled anyway) one. There simply must not >> be a 'case 1' there, and the insn_decode table should be changed >> accordingly. > >You mean the following fragment? > > if ( !(decode & HAS_MODRM) ) > { > switch ( decode & 7 ) > { > case 1: > offset = (long)(*(char *)pb); > goto skip_modrm; Yes. >It is correct -- sign extends the 8-bit offset to a signed long as we >require. The instruction only contains a single-byte offset -- it >isn't stored as 16 or 32 bits. I don't think so. The instructions this refers to are (opcodes A1 and A3) movb symbol, %al movb %al, symbol Clearly, they take a full (16-/32-bit) address but operate on a byte at that address (they would be useless if they operated only on an 8-bit sign-extended address). Having a second look at this reveals another dangerous thing: The code here directly derefences pb, whereas all other instances of references through ip correctly use get_user. Finally, in the case I'm still missing something here and the 'case 1' is needed, then an operation like (long)*(char *) is rather dangerous, because you depend on the (implementation defined) signedness of char. You'd want to code (long)*(signed char *) instead. Jan ------------------------------------------------------- The SF.Net email is sponsored by: Beat the post-holiday blues Get a FREE limited edition SourceForge.net t-shirt from ThinkGeek. It's fun and FREE -- well, almost....http://www.thinkgeek.com/sfshirt _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.sourceforge.net/lists/listinfo/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |