x86/fault: Fix sign-extend unintended sign extension

Colin Ian King · KAGA-KOKO · commit 5ccd35287eda · 2019-01-29T21:58:59.000+01:00
show_ldttss() shifts desc.base2 by 24 bit, but base2 is 8 bits of a bitfield in a u16. Due to the really great idea of integer promotion in C99 base2 is promoted to an int, because that's the standard defined behaviour when all values which can be represented by base2 fit into an int. Now if bit 7 is set in desc.base2 the result of the shift left by 24 makes the resulting integer negative and the following conversion to unsigned long legitmately sign extends first causing the upper bits 32 bits to be set in the result. Fix this by casting desc.base2 to unsigned long before the shift. Detected by CoverityScan, CID#1475635 ("Unintended sign extension") [ tglx: Reworded the changelog a bit as I actually had to lookup the standard (again) to decode the original one. ] Fixes: a1a371c ("x86/fault: Decode page fault OOPSes better") Signed-off-by: Colin Ian King <colin.king@canonical.com> Signed-off-by: Thomas Gleixner <tglx@linutronix.de> Cc: Dave Hansen <dave.hansen@linux.intel.com> Cc: Andy Lutomirski <luto@kernel.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Borislav Petkov <bp@alien8.de> Cc: "H . Peter Anvin" <hpa@zytor.com> Cc: kernel-janitors@vger.kernel.org Link: https://lkml.kernel.org/r/20181222191116.21831-1-colin.king@canonical.com
diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
@@ -595,7 +595,7 @@ static void show_ldttss(const struct desc_ptr *gdt, const char *name, u16 index)
 		return;
 	}
 
-	addr = desc.base0 | (desc.base1 << 16) | (desc.base2 << 24);
+	addr = desc.base0 | (desc.base1 << 16) | ((unsigned long)desc.base2 << 24);
 #ifdef CONFIG_X86_64
 	addr |= ((u64)desc.base3 << 32);
 #endif

Original file line number	Diff line number	Diff line change
`@@ -595,7 +595,7 @@ static void show_ldttss(const struct desc_ptr gdt, const char name, u16 index)`
`595`	`595`	`return;`
`596`	`596`	`}`
`597`	`597`
`598`		`- addr = desc.base0 \| (desc.base1 << 16) \| (desc.base2 << 24);`
	`598`	`+ addr = desc.base0 \| (desc.base1 << 16) \| ((unsigned long)desc.base2 << 24);`
`599`	`599`	`#ifdef CONFIG_X86_64`
`600`	`600`	`addr \|= ((u64)desc.base3 << 32);`
`601`	`601`	`#endif`