[PATCH] x86/insn-eval: Fix signedness bug in segment selector handling

[PATCH] x86/insn-eval: Fix signedness bug in segment selector handling

[Uros Bizjak]

get_segment_selector() returns a short, while callers use the pattern:

	short sel = get_segment_selector(...);
	if (sel < 0)
		return -ERR;

Segment selectors are 16-bit values, but storing them in a signed
16-bit type means values with the MSB set (>= 0x8000) become negative.
This causes valid selectors to be misinterpreted as errors by the
'sel < 0' check.

Change get_segment_selector() to return int and update all call sites
to use 'int sel' to avoid unintended sign extension and keep error
handling via negative return values correct.

Additionally, remove the explicit & 0xffff masking when reading
segment registers. The compiler already zero-extends
unsigned 16-bit values when loading them into a
wider type, so the masking is redundant.

With this change, valid segment selectors are no longer
confused with error returns.

Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
Cc: Thomas Gleixner <tglx@kernel.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
---
 arch/x86/lib/insn-eval.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/arch/x86/lib/insn-eval.c b/arch/x86/lib/insn-eval.c
index e03eeec55cfe..b8847ce0b282 100644
--- a/arch/x86/lib/insn-eval.c
+++ b/arch/x86/lib/insn-eval.c
@@ -340,18 +340,18 @@ static int resolve_seg_reg(struct insn *insn, struct pt_regs *regs, int regoff)
  *
  * -EINVAL on error.
  */
-static short get_segment_selector(struct pt_regs *regs, int seg_reg_idx)
+static int get_segment_selector(struct pt_regs *regs, int seg_reg_idx)
 {
-	unsigned short sel;
+	unsigned int sel;
 
 #ifdef CONFIG_X86_64
 	switch (seg_reg_idx) {
 	case INAT_SEG_REG_IGNORE:
 		return 0;
 	case INAT_SEG_REG_CS:
-		return (unsigned short)(regs->cs & 0xffff);
+		return regs->cs;
 	case INAT_SEG_REG_SS:
-		return (unsigned short)(regs->ss & 0xffff);
+		return regs->ss;
 	case INAT_SEG_REG_DS:
 		savesegment(ds, sel);
 		return sel;
@@ -373,9 +373,9 @@ static short get_segment_selector(struct pt_regs *regs, int seg_reg_idx)
 	if (v8086_mode(regs)) {
 		switch (seg_reg_idx) {
 		case INAT_SEG_REG_CS:
-			return (unsigned short)(regs->cs & 0xffff);
+			return regs->cs;
 		case INAT_SEG_REG_SS:
-			return (unsigned short)(regs->ss & 0xffff);
+			return regs->ss;
 		case INAT_SEG_REG_DS:
 			return vm86regs->ds;
 		case INAT_SEG_REG_ES:
@@ -392,15 +392,15 @@ static short get_segment_selector(struct pt_regs *regs, int seg_reg_idx)
 
 	switch (seg_reg_idx) {
 	case INAT_SEG_REG_CS:
-		return (unsigned short)(regs->cs & 0xffff);
+		return regs->cs;
 	case INAT_SEG_REG_SS:
-		return (unsigned short)(regs->ss & 0xffff);
+		return regs->ss;
 	case INAT_SEG_REG_DS:
-		return (unsigned short)(regs->ds & 0xffff);
+		return regs->ds;
 	case INAT_SEG_REG_ES:
-		return (unsigned short)(regs->es & 0xffff);
+		return regs->es;
 	case INAT_SEG_REG_FS:
-		return (unsigned short)(regs->fs & 0xffff);
+		return regs->fs;
 	case INAT_SEG_REG_GS:
 		savesegment(gs, sel);
 		return sel;
@@ -688,7 +688,7 @@ static bool get_desc(struct desc_struct *out, unsigned short sel)
 unsigned long insn_get_seg_base(struct pt_regs *regs, int seg_reg_idx)
 {
 	struct desc_struct desc;
-	short sel;
+	int sel;
 
 	sel = get_segment_selector(regs, seg_reg_idx);
 	if (sel < 0)
@@ -756,7 +756,7 @@ static unsigned long get_seg_limit(struct pt_regs *regs, int seg_reg_idx)
 {
 	struct desc_struct desc;
 	unsigned long limit;
-	short sel;
+	int sel;
 
 	sel = get_segment_selector(regs, seg_reg_idx);
 	if (sel < 0)
@@ -803,7 +803,7 @@ static unsigned long get_seg_limit(struct pt_regs *regs, int seg_reg_idx)
 int insn_get_code_seg_params(struct pt_regs *regs)
 {
 	struct desc_struct desc;
-	short sel;
+	int sel;
 
 	if (v8086_mode(regs))
 		/* Address and operand size are both 16-bit. */
-- 
2.53.0