On Mon, Feb 25, 2019 at 08:29:12AM -0800, Andy Lutomirski wrote:
> > diff --git a/arch/x86/ia32/ia32_signal.c b/arch/x86/ia32/ia32_signal.c
> > index 321fe5f5d0e9..e04eeeddcc35 100644
> > --- a/arch/x86/ia32/ia32_signal.c
> > +++ b/arch/x86/ia32/ia32_signal.c
> > @@ -53,17 +53,16 @@
> > #define GET_SEG(seg) ({ \
> > unsigned short tmp; \
> > get_user_ex(tmp, &sc->seg); \
> > - tmp; \
> > + tmp | 3; \
> > })
> >
>
> Drop this part.
>
> > #define COPY_SEG_CPL3(seg) do { \
> > - regs->seg = GET_SEG(seg) | 3; \
> > + regs->seg = GET_SEG(seg); \
> > } while (0)
>
> And this.
>
> Unfortunately, whether we want the | 3 varies by segment. For FS and
> GS, we definitely don’t want it, since 0 is a common and important
> value, and 3 is a deeply screwed up value. (3 is legal to *write* to
> GS, and it sticks, but IRET silently changes it to 0, because the
> original 386 designers (I assume) confused themselves.
> >
> > #define RELOAD_SEG(seg) { \
> > - unsigned int pre = GET_SEG(seg); \
> > + unsigned int pre = (seg); \
> > unsigned int cur = get_user_seg(seg); \
> > - pre |= 3; \
And here ?
> > if (pre != cur) \
> > set_user_seg(seg, pre); \
> > }
Thing is; afaict the current code _always_ does |3 on any GET_SET()
result.
If you want to change that; I'm fine with that, but lets not do that in
this same patch.
So then?
diff --git a/arch/x86/ia32/ia32_signal.c b/arch/x86/ia32/ia32_signal.c
index 321fe5f5d0e9..4d5fcd47ab75 100644
--- a/arch/x86/ia32/ia32_signal.c
+++ b/arch/x86/ia32/ia32_signal.c
@@ -61,9 +61,8 @@
} while (0)
#define RELOAD_SEG(seg) { \
- unsigned int pre = GET_SEG(seg); \
+ unsigned int pre = (seg) | 3; \
unsigned int cur = get_user_seg(seg); \
- pre |= 3; \
if (pre != cur) \
set_user_seg(seg, pre); \
}
@@ -72,6 +71,7 @@ static int ia32_restore_sigcontext(struct pt_regs *regs,
struct sigcontext_32 __user *sc)
{
unsigned int tmpflags, err = 0;
+ u16 gs, fs, es, ds;
void __user *buf;
u32 tmp;
@@ -79,16 +79,10 @@ static int ia32_restore_sigcontext(struct pt_regs *regs,
current->restart_block.fn = do_no_restart_syscall;
get_user_try {
- /*
- * Reload fs and gs if they have changed in the signal
- * handler. This does not handle long fs/gs base changes in
- * the handler, but does not clobber them at least in the
- * normal case.
- */
- RELOAD_SEG(gs);
- RELOAD_SEG(fs);
- RELOAD_SEG(ds);
- RELOAD_SEG(es);
+ gs = GET_SEG(gs);
+ fs = GET_SEG(fs);
+ ds = GET_SEG(ds);
+ es = GET_SEG(es);
COPY(di); COPY(si); COPY(bp); COPY(sp); COPY(bx);
COPY(dx); COPY(cx); COPY(ip); COPY(ax);
@@ -106,6 +100,17 @@ static int ia32_restore_sigcontext(struct pt_regs *regs,
buf = compat_ptr(tmp);
} get_user_catch(err);
+ /*
+ * Reload fs and gs if they have changed in the signal
+ * handler. This does not handle long fs/gs base changes in
+ * the handler, but does not clobber them at least in the
+ * normal case.
+ */
+ RELOAD_SEG(gs);
+ RELOAD_SEG(fs);
+ RELOAD_SEG(ds);
+ RELOAD_SEG(es);
+
err |= fpu__restore_sig(buf, 1);
force_iret();
