[PATCH v2 1/1] x86/ia32: Normalize any null selector value to 0

[PATCH v2 1/1] x86/ia32: Normalize any null selector value to 0

[Xin Li (Intel)]

The first GDT descriptor is reserved as 'null descriptor'.  As bits 0
and 1 of a segment selector, i.e., the DPL bits, are NOT used to index
GDT, selector values 0~3 all point to the null descriptor, thus values
0, 1, 2 and 3 are all valid null selector values.

Furthermore IRET zeros ES, FS, GS, and DS segment registers if any of
them is found to have any null selector value, essentially making 0 a
preferred null selector value.

reload_segments() _unconditionally_ sets ES, FS, GS and DS segement
registers' DPL bits, which is unnecessary for null selector values,
since they will be zeroed later by IRET as used to return to userspace.

Unlike IRET, ERETU, introduced with FRED to return to userspace from
kernel, does not make any of DS, ES, FS, or GS null if it is found to
have DPL < 3.  Thus when FRED is enabled, a userspace segment register
ends up with 3 in it even when its initial value is 0 before entering
kernel, which fails the 32-bit sigreturn selftest.

Normalize any null selector value, 0~3, to 0, while for any non-null
selector, sets both DPL bits to force it to be a user level segment
selector.

Apply the same normalization logic in a 32bit kernel as well.

Signed-off-by: Xin Li (Intel) <xin@zytor.com>
---

Changes since v1:
* Normalize non-zero null selector values to 0 (Eric W. Biederman).
* Apply the same normalization logic in a 32bit kernel (Eric W.
  Biederman).
---
 arch/x86/kernel/signal_32.c | 58 +++++++++++++++++++++++++++----------
 1 file changed, 42 insertions(+), 16 deletions(-)

diff --git a/arch/x86/kernel/signal_32.c b/arch/x86/kernel/signal_32.c
index ef654530bf5a..73230892362f 100644
--- a/arch/x86/kernel/signal_32.c
+++ b/arch/x86/kernel/signal_32.c
@@ -33,25 +33,51 @@
 #include <asm/smap.h>
 #include <asm/gsseg.h>
 
+/*
+ * The first GDT descriptor is reserved as 'null descriptor'.  As bits 0
+ * and 1 of a segment selector, i.e., the DPL bits, are NOT used to index
+ * GDT, selector values 0~3 all point to the null descriptor, thus values
+ * 0, 1, 2 and 3 are all valid null selector values.
+ *
+ * Furthermore IRET zeros ES, FS, GS, and DS segment registers if any of
+ * them is found to have any null selector value, essentially making 0 a
+ * preferred null selector value.
+ *
+ * Normalizes any null selector value, 0~3, to 0, while for any non-null
+ * selector, sets both DPL bits to force it to be a user level segment
+ * selector.
+ */
+static inline u16 normalize_useg_selector(u16 sel)
+{
+	return sel <= 3 ? 0 : sel | 3;
+}
+
 #ifdef CONFIG_IA32_EMULATION
 #include <asm/unistd_32_ia32.h>
 
 static inline void reload_segments(struct sigcontext_32 *sc)
 {
-	unsigned int cur;
+	unsigned int new, cur;
 
+	new = normalize_useg_selector(sc->gs);
 	savesegment(gs, cur);
-	if ((sc->gs | 0x03) != cur)
-		load_gs_index(sc->gs | 0x03);
-	savesegment(fs, cur);
-	if ((sc->fs | 0x03) != cur)
-		loadsegment(fs, sc->fs | 0x03);
-	savesegment(ds, cur);
-	if ((sc->ds | 0x03) != cur)
-		loadsegment(ds, sc->ds | 0x03);
-	savesegment(es, cur);
-	if ((sc->es | 0x03) != cur)
-		loadsegment(es, sc->es | 0x03);
+	cur = normalize_useg_selector(cur);
+	if (new != cur)
+		load_gs_index(new);
+
+#define RELOAD_USEG_SELECTOR(seg) {			\
+	new = normalize_useg_selector(sc->seg);		\
+	savesegment(seg, cur);				\
+	cur = normalize_useg_selector(cur);		\
+	if (new != cur)					\
+		loadsegment(seg, new);			\
+}
+
+	RELOAD_USEG_SELECTOR(fs);
+	RELOAD_USEG_SELECTOR(ds);
+	RELOAD_USEG_SELECTOR(es);
+
+#undef RELOAD_USEG_SELECTOR
 }
 
 #define sigset32_t			compat_sigset_t
@@ -113,10 +139,10 @@ static bool ia32_restore_sigcontext(struct pt_regs *regs,
 	 */
 	reload_segments(&sc);
 #else
-	loadsegment(gs, sc.gs);
-	regs->fs = sc.fs;
-	regs->es = sc.es;
-	regs->ds = sc.ds;
+	loadsegment(gs, normalize_useg_selector(sc.gs));
+	regs->fs = normalize_useg_selector(sc.fs);
+	regs->es = normalize_useg_selector(sc.es);
+	regs->ds = normalize_useg_selector(sc.ds);
 #endif
 
 	return fpu__restore_sig(compat_ptr(sc.fpstate), 1);

base-commit: d5546ad1b9b17446e738741435aee92ee49fc4df
-- 
2.47.0

Re: [PATCH v2 1/1] x86/ia32: Normalize any null selector value to 0

[Andrew Cooper]

> The first GDT descriptor is reserved as 'null descriptor'.  As bits 0
> and 1 of a segment selector, i.e., the DPL bits, are NOT used to index

That's RPL in segment selector, not DPL.  Same correction is needed in
the code comments.

> GDT, selector values 0~3 all point to the null descriptor, thus values
> 0, 1, 2 and 3 are all valid null selector values.
>
> Furthermore IRET zeros ES, FS, GS, and DS segment registers if any of
> them is found to have any null selector value, essentially making 0 a
> preferred null selector value.

Zeroing of RPL in null selectors is an information leak in pre-FRED
systems.  Userspace can spot any interrupt/exception by loading a
nonzero NULL selector, and waiting for it to drop to zero.

Userspace should not be able to do this; Andy and I lobbied for this
during the design of FRED, and Intel agreed.

Right now, this change is codifying the problem behaviour we were trying
to fix out under FRED.

Under FRED, if userspace loads e.g. 2 into a selector, it should remain
2 until userspace changes it to something else.

~Andrew