lkml.org 
[lkml]   [1998]   [Aug]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [patch] Re: [2.1.117] GPF in APM while using WINE


On Mon, 24 Aug 1998, Linus Torvalds wrote:

> Yes, the reason this didn't show up before was that we had interrupts
> effectively disabled for the whole context switch (simply because we did
> it with a single instruction and thus the CPU wouldn't accept interrupts
> until it was all done..)
>
> > - make the switch_to() code safe wrt segment saves without
> > disabling IRQs, by zeroing out segment registers. (zeroing
> > segment registers takes exactly 1 cycle on my PII)
>
> This looks like a reasonable solution, especially as we only need to do
> this when we reload the LDT. However...
>
> > is there any other place in the kernel where segment registers might hold
> > invalid contents?
>
> There are cases where a user can force a bad segment into %fs and %gs by
> loading a new LDT - the kernel won't care. It has the logic to cleanly
> reload zero segments in the normal paths.

Indeed.

>
> I would suggest that instead of your patch to __switch_to(), the APM code
> should probably re-load %fs and %gs with
>
> loadsegment(fs, value);
> loadsegment(gs, value);
>
> which correctly takes the GP trap and loads a zero if the value was bad
> using the normal exception mechanism (ie no overhead at all except for the
> case where it actually traps).

Except for one thing, saving %fs and %gs should be done _before_ loading
the new ldt, otherwise %fs and/or %gs can be automagically zeroed in an
APM interrupt (or any other which happens to use these segment
registers) and saved as such in task struct (with interesting side
effects on Wine and friends ;)). Ingo's patch is right in this respect.

> That will work correctly for both the case where a interrupt happened
> while the segments hadn't yet been updated, AND for the case (which can be
> synchronous) where a user had invalidated the segment pointed to by
> %fs/%gs.
>

But this was not my main point, now that all segment register loads and
even the iret is protected from exceptions, it might be time to remove
the bogus checks for selectors in restore_sigcontext. Hence the attached
patch (perhaps to aggressive, I'll admit). It will hopefully make
signal return faster and some thread libraries happier.

Gabriel.
--- linux-2.1.117-vanilla/arch/i386/kernel/signal.c Mon Aug 24 10:56:01 1998
+++ linux-2.1.117/arch/i386/kernel/signal.c Tue Aug 25 09:52:14 1998
@@ -184,26 +184,17 @@

#define COPY_SEG(seg) \
{ unsigned short tmp; \
- err |= __get_user(tmp, &sc->seg); \
- if ((tmp & 0xfffc) /* not a NULL selectors */ \
- && (tmp & 0x4) != 0x4 /* not a LDT selector */ \
- && (tmp & 3) != 3) /* not a RPL3 GDT selector */ \
- goto badframe; \
+ err |= __get_user(tmp, &sc->seg); \
regs->x##seg = tmp; }

#define COPY_SEG_STRICT(seg) \
{ unsigned short tmp; \
err |= __get_user(tmp, &sc->seg); \
- if ((tmp & 0xfffc) && (tmp & 3) != 3) goto badframe; \
- regs->x##seg = tmp; }
+ regs->x##seg = tmp|3; }

#define GET_SEG(seg) \
{ unsigned short tmp; \
err |= __get_user(tmp, &sc->seg); \
- if ((tmp & 0xfffc) /* not a NULL selectors */ \
- && (tmp & 0x4) != 0x4 /* not a LDT selector */ \
- && (tmp & 3) != 3) /* not a RPL3 GDT selector */ \
- goto badframe; \
loadsegment(seg,tmp); }

GET_SEG(gs);
\
 
 \ /
  Last update: 2005-03-22 13:44    [from the cache]
©2003-2011 Jasper Spaans