linux-kernel - [PATCH v9 01/17] x86/ptrace: Prevent ptrace from clearing the FS/GS selector

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <1570212969-21888-2-git-send-email-chang.seok.bae@intel.com>
Date:   Fri,  4 Oct 2019 11:15:53 -0700
From:   "Chang S. Bae" <chang.seok.bae@...el.com>
To:     linux-kernel@...r.kernel.org, tglx@...utronix.de, bp@...en8.de,
        luto@...nel.org
Cc:     hpa@...or.com, dave.hansen@...el.com, tony.luck@...el.com,
        ak@...ux.intel.com, ravi.v.shankar@...el.com,
        chang.seok.bae@...el.com
Subject: [PATCH v9 01/17] x86/ptrace: Prevent ptrace from clearing the FS/GS selector

When a ptracer writes a ptracee's FS/GS base with a different value, the
selector is also cleared. This behavior is not correct as the selector
should be preserved.

Update only the base value and leave the selector intact. To simplify the
code further remove the conditional checking for the same value as this
code is not performance-critical.

The only recognizable downside of this change is when the selector is
already nonzero on write. The base will be reloaded according to the
selector. But the case is highly unexpected in real usages.

Suggested-by: Andy Lutomirski <luto@...nel.org>
Signed-off-by: Chang S. Bae <chang.seok.bae@...el.com>
Reviewed-by: Tony Luck <tony.luck@...el.com>
Cc: Thomas Gleixner <tglx@...utronix.de>
Cc: Borislav Petkov <bp@...en8.de>
Cc: Andy Lutomirski <luto@...nel.org>
Cc: H. Peter Anvin <hpa@...or.com>
Cc: Dave Hansen <dave.hansen@...el.com>
Cc: Tony Luck <tony.luck@...el.com>
Cc: Andi Kleen <ak@...ux.intel.com>
---

Changes from v8: none

Changes from v7:
* Fixed to call correct helper functions
* Massaged changelog by Thomas
* Used '[FS|GS] base' consistently, instead of '[FS|GS]BASE'
---
 arch/x86/kernel/ptrace.c | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/arch/x86/kernel/ptrace.c b/arch/x86/kernel/ptrace.c
index 3c5bbe8..df222e2 100644
--- a/arch/x86/kernel/ptrace.c
+++ b/arch/x86/kernel/ptrace.c
@@ -370,22 +370,12 @@ static int putreg(struct task_struct *child,
 	case offsetof(struct user_regs_struct,fs_base):
 		if (value >= TASK_SIZE_MAX)
 			return -EIO;
-		/*
-		 * When changing the FS base, use do_arch_prctl_64()
-		 * to set the index to zero and to set the base
-		 * as requested.
-		 */
-		if (child->thread.fsbase != value)
-			return do_arch_prctl_64(child, ARCH_SET_FS, value);
+		x86_fsbase_write_task(child, value);
 		return 0;
 	case offsetof(struct user_regs_struct,gs_base):
-		/*
-		 * Exactly the same here as the %fs handling above.
-		 */
 		if (value >= TASK_SIZE_MAX)
 			return -EIO;
-		if (child->thread.gsbase != value)
-			return do_arch_prctl_64(child, ARCH_SET_GS, value);
+		x86_gsbase_write_task(child, value);
 		return 0;
 #endif
 	}
-- 
2.7.4