WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel] NPTL/TLS segment flipping code problem

To: <Keir.Fraser@xxxxxxxxxxxx>
Subject: Re: [Xen-devel] NPTL/TLS segment flipping code problem
From: "Jan Beulich" <JBeulich@xxxxxxxxxx>
Date: Wed, 12 Jan 2005 14:25:38 +0100
Cc: <xen-devel@xxxxxxxxxxxxxxxxxxxxx>
Delivery-date: Thu, 13 Jan 2005 09:14:02 +0000
Envelope-to: xen+James.Bulpin@xxxxxxxxxxxx
List-archive: <http://sourceforge.net/mailarchive/forum.php?forum=xen-devel>
List-help: <mailto:xen-devel-request@lists.sourceforge.net?subject=help>
List-id: List for Xen developers <xen-devel.lists.sourceforge.net>
List-post: <mailto:xen-devel@lists.sourceforge.net>
List-subscribe: <https://lists.sourceforge.net/lists/listinfo/xen-devel>, <mailto:xen-devel-request@lists.sourceforge.net?subject=subscribe>
List-unsubscribe: <https://lists.sourceforge.net/lists/listinfo/xen-devel>, <mailto:xen-devel-request@lists.sourceforge.net?subject=unsubscribe>
Sender: xen-devel-admin@xxxxxxxxxxxxxxxxxxxxx
>> 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