Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.31 as permitted sender) client-ip=23.128.96.31;
From:   Brian Gerst <brgerst@gmail.com>
To:     linux-kernel@vger.kernel.org, x86@kernel.org
Cc:     Ingo Molnar <mingo@kernel.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Borislav Petkov <bp@alien8.de>,
        "H . Peter Anvin" <hpa@zytor.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Linus Torvalds <torvalds@linuxfoundation.org>,
        Brian Gerst <brgerst@gmail.com>, Michal Luczaj <mhal@rbox.co>
Subject: [PATCH 2/3] x86/ptrace: Reject system segements
Date:   Wed, 13 Dec 2023 11:34:42 -0500
Message-ID: <20231213163443.70490-3-brgerst@gmail.com>
In-Reply-To: <20231213163443.70490-1-brgerst@gmail.com>
References: <20231213163443.70490-1-brgerst@gmail.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

Do not allow system segments (TSS and LDT) from being poked into segment
registers via ptrace.  Loading these segments into a segment register
normally results in a general protection fault.  But in the case of
ptrace, setting CS or SS to a system segment will cause IRET to fault.
This then results in the instruction decoder attempting to use the
invalid segment.  This can be avoided by rejecting system segments in
PTRACE_SETREGS.

Signed-off-by: Brian Gerst <brgerst@gmail.com>
Reported-By: Michal Luczaj <mhal@rbox.co>
Link: https://lore.kernel.org/lkml/20231206004654.2986026-1-mhal@rbox.co/
---
 arch/x86/include/asm/segment.h | 11 +++++++++++
 arch/x86/kernel/ptrace.c       | 12 ++----------
 2 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/arch/x86/include/asm/segment.h b/arch/x86/include/asm/segment.h
index a155843d0c37..ede1fa5aa4cc 100644
--- a/arch/x86/include/asm/segment.h
+++ b/arch/x86/include/asm/segment.h
@@ -359,6 +359,17 @@ static inline void __loadsegment_fs(unsigned short value)
 #define savesegment(seg, value)				\
 	asm("mov %%" #seg ",%0":"=r" (value) : : "memory")
 
+/*
+ * Determines whether a value may be installed in a segment register.
+ */
+static inline bool valid_user_selector(u16 value)
+{
+	if (unlikely(!(value & SEGMENT_TI_MASK) && value >= (GDT_SYSTEM_START * 8)))
+		return false;
+
+	return likely(value == 0 || (value & SEGMENT_RPL_MASK) == USER_RPL);
+}
+
 #endif /* !__ASSEMBLY__ */
 #endif /* __KERNEL__ */
 
diff --git a/arch/x86/kernel/ptrace.c b/arch/x86/kernel/ptrace.c
index 095f04bdabdc..4c3a2278691e 100644
--- a/arch/x86/kernel/ptrace.c
+++ b/arch/x86/kernel/ptrace.c
@@ -162,14 +162,6 @@ const char *regs_query_register_name(unsigned int offset)
 				  X86_EFLAGS_DF | X86_EFLAGS_OF |	\
 				  X86_EFLAGS_RF | X86_EFLAGS_AC))
 
-/*
- * Determines whether a value may be installed in a segment register.
- */
-static inline bool invalid_selector(u16 value)
-{
-	return unlikely(value != 0 && (value & SEGMENT_RPL_MASK) != USER_RPL);
-}
-
 #ifdef CONFIG_X86_32
 
 #define FLAG_MASK		FLAG_MASK_32
@@ -206,7 +198,7 @@ static int set_segment_reg(struct task_struct *task,
 	/*
 	 * The value argument was already truncated to 16 bits.
 	 */
-	if (invalid_selector(value))
+	if (!valid__user_selector(value))
 		return -EIO;
 
 	/*
@@ -296,7 +288,7 @@ static int set_segment_reg(struct task_struct *task,
 	/*
 	 * The value argument was already truncated to 16 bits.
 	 */
-	if (invalid_selector(value))
+	if (!valid_user_selector(value))
 		return -EIO;
 
 	/*
-- 
2.43.0