From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id D5F3EC433F5 for ; Mon, 3 Oct 2022 19:02:05 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 4A2726B0071; Mon, 3 Oct 2022 15:02:05 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 4522B6B0073; Mon, 3 Oct 2022 15:02:05 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 2CA708E0001; Mon, 3 Oct 2022 15:02:05 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 196BB6B0071 for ; Mon, 3 Oct 2022 15:02:05 -0400 (EDT) Received: from smtpin24.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 90EC2AAF5C for ; Mon, 3 Oct 2022 19:02:04 +0000 (UTC) X-FDA: 79980558168.24.4AED3CD Received: from mail-pj1-f45.google.com (mail-pj1-f45.google.com [209.85.216.45]) by imf01.hostedemail.com (Postfix) with ESMTP id 8F92D4001E for ; Mon, 3 Oct 2022 19:02:02 +0000 (UTC) Received: by mail-pj1-f45.google.com with SMTP id v10-20020a17090a634a00b00205e48cf845so16108013pjs.4 for ; Mon, 03 Oct 2022 12:02:02 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date; bh=jpaG3SrEmgSBAm16nhnpNwprOxeW2H3keLquV2/wBXQ=; b=gEE5lkKG7BExD++0qu/8MiwYA2SzP1xrc3xdMYGJ1kuzuMHTVOAP1I1fgKeMZMdIgO fjwcaKbKXaJuAOkAhjE9NgnG53BafRqFmKTVcg0QW8EknJ2kwabXqVFJxiVqBom2y3t3 IMeZGnsbBh1FGNURLn+pM7KSOLcfZzJycr5do= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date; bh=jpaG3SrEmgSBAm16nhnpNwprOxeW2H3keLquV2/wBXQ=; b=h+0NYwG32J/S1y45HrOfW4JpMTq4kE072tPRwKf9sQZ1SEYR91j2megnY046gtfW4f oEBILBDduzhQkUlrQjV9J/+J4Rh5Ix35+5YYkfSvyF+tFVuYhuL7FMNm61ffl+49uGhp JxtozcKC/ivwSIGaCXrz3r1ISo+eOZwaV+oAd5TIz3vY5tq0D4XCtFuTaiCUyF/0lg3L uiOO9D0UFNkSW+QxRTZP1Ne5klEfHNCmmm18vRHW+89kl9gar5/Y6LlJ22kQJIJ4bZD3 Ow7p4OmoTYQXpZLDXfgS8usaRxxkyuoNRrTUE7n7oi28sSBPA2M2EUbvASk8uyQJW70O 1rwg== X-Gm-Message-State: ACrzQf13AUhLz/aUAWdKDklIhDLVz9aps5YvqrWrp1gbga3RCeduUbXJ aJ78nf5evbPxEkpWSnQhnSK/uA== X-Google-Smtp-Source: AMsMyM7iEJ6ZXFVkmMaqs2OMbOTsAAmyiAUm7DQD5ClE0HPGFpaU7EluMkgUVp/2nJ+cWAmuLqN/dA== X-Received: by 2002:a17:902:d2c6:b0:17f:592b:35dd with SMTP id n6-20020a170902d2c600b0017f592b35ddmr6935870plc.172.1664823721399; Mon, 03 Oct 2022 12:02:01 -0700 (PDT) Received: from www.outflux.net (smtp.outflux.net. [198.145.64.163]) by smtp.gmail.com with ESMTPSA id s17-20020a170903215100b00173411a4385sm7536367ple.43.2022.10.03.12.02.00 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 03 Oct 2022 12:02:00 -0700 (PDT) Date: Mon, 3 Oct 2022 12:01:59 -0700 From: Kees Cook To: Rick Edgecombe Cc: x86@kernel.org, "H . Peter Anvin" , Thomas Gleixner , Ingo Molnar , linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org, linux-mm@kvack.org, linux-arch@vger.kernel.org, linux-api@vger.kernel.org, Arnd Bergmann , Andy Lutomirski , Balbir Singh , Borislav Petkov , Cyrill Gorcunov , Dave Hansen , Eugene Syromiatnikov , Florian Weimer , "H . J . Lu" , Jann Horn , Jonathan Corbet , Mike Kravetz , Nadav Amit , Oleg Nesterov , Pavel Machek , Peter Zijlstra , Randy Dunlap , "Ravi V . Shankar" , Weijiang Yang , "Kirill A . Shutemov" , joao.moreira@intel.com, John Allen , kcc@google.com, eranian@google.com, rppt@kernel.org, jamorris@linux.microsoft.com, dethoma@microsoft.com Subject: Re: [PATCH v2 23/39] x86: Introduce userspace API for CET enabling Message-ID: <202210031141.0E0DE2CAEE@keescook> References: <20220929222936.14584-1-rick.p.edgecombe@intel.com> <20220929222936.14584-24-rick.p.edgecombe@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220929222936.14584-24-rick.p.edgecombe@intel.com> ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1664823722; a=rsa-sha256; cv=none; b=iJPVaNKY/lmR3J03Unuee4tM7pjlpDxwWv9ehUBJ+nbpvK4Rs84p9CUCwj1/kMOA2lD4DG izSIjdVy/3Gz5hA5kvwQD/eCUHzlspoLE/4U8IoTqOwDf5/QbKQNAFumvOQnY/4Ki/14fu 5GYO9J7GKmULxk9OJzjqK8Tk9wvcp4s= ARC-Authentication-Results: i=1; imf01.hostedemail.com; dkim=pass header.d=chromium.org header.s=google header.b=gEE5lkKG; spf=pass (imf01.hostedemail.com: domain of keescook@chromium.org designates 209.85.216.45 as permitted sender) smtp.mailfrom=keescook@chromium.org; dmarc=pass (policy=none) header.from=chromium.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1664823722; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=jpaG3SrEmgSBAm16nhnpNwprOxeW2H3keLquV2/wBXQ=; b=Yeiz7g0YdV8ueCDWDHml7cGaiJVXyJN+ZJWl8u4x2mdYyURXukqGbIqXk5Rzq/HLZVgL64 XG5FiINiYgw3emDY1/BmC1SdZ1d4KTomzI80Og3QorZlwUap/fkuNgluDLiSzmPSDLb9+O JFbNWVcr6xvsqyWECJxFcCICnwKSMKY= X-Rspamd-Server: rspam03 X-Rspamd-Queue-Id: 8F92D4001E X-Rspam-User: Authentication-Results: imf01.hostedemail.com; dkim=pass header.d=chromium.org header.s=google header.b=gEE5lkKG; spf=pass (imf01.hostedemail.com: domain of keescook@chromium.org designates 209.85.216.45 as permitted sender) smtp.mailfrom=keescook@chromium.org; dmarc=pass (policy=none) header.from=chromium.org X-Stat-Signature: jqpk9p6tswir5jw3hictp5rtqn66q7xs X-HE-Tag: 1664823722-110142 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Thu, Sep 29, 2022 at 03:29:20PM -0700, Rick Edgecombe wrote: > From: "Kirill A. Shutemov" > > Add three new arch_prctl() handles: > > - ARCH_CET_ENABLE/DISABLE enables or disables the specified > feature. Returns 0 on success or an error. > > - ARCH_CET_LOCK prevents future disabling or enabling of the > specified feature. Returns 0 on success or an error > > The features are handled per-thread and inherited over fork(2)/clone(2), > but reset on exec(). > > This is preparation patch. It does not impelement any features. typo: "implement" > > Signed-off-by: Kirill A. Shutemov > [tweaked with feedback from tglx] > Co-developed-by: Rick Edgecombe > Signed-off-by: Rick Edgecombe > > --- > > v2: > - Only allow one enable/disable per call (tglx) > - Return error code like a normal arch_prctl() (Alexander Potapenko) > - Make CET only (tglx) > > arch/x86/include/asm/cet.h | 20 ++++++++++++++++ > arch/x86/include/asm/processor.h | 3 +++ > arch/x86/include/uapi/asm/prctl.h | 6 +++++ > arch/x86/kernel/process.c | 4 ++++ > arch/x86/kernel/process_64.c | 5 +++- > arch/x86/kernel/shstk.c | 38 +++++++++++++++++++++++++++++++ > 6 files changed, 75 insertions(+), 1 deletion(-) > create mode 100644 arch/x86/include/asm/cet.h > create mode 100644 arch/x86/kernel/shstk.c > > diff --git a/arch/x86/include/asm/cet.h b/arch/x86/include/asm/cet.h > new file mode 100644 > index 000000000000..0fa4dbc98c49 > --- /dev/null > +++ b/arch/x86/include/asm/cet.h > @@ -0,0 +1,20 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +#ifndef _ASM_X86_CET_H > +#define _ASM_X86_CET_H > + > +#ifndef __ASSEMBLY__ > +#include > + > +struct task_struct; > + > +#ifdef CONFIG_X86_SHADOW_STACK > +long cet_prctl(struct task_struct *task, int option, > + unsigned long features); > +#else > +static inline long cet_prctl(struct task_struct *task, int option, > + unsigned long features) { return -EINVAL; } > +#endif /* CONFIG_X86_SHADOW_STACK */ > + > +#endif /* __ASSEMBLY__ */ > + > +#endif /* _ASM_X86_CET_H */ > diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h > index 356308c73951..a92bf76edafe 100644 > --- a/arch/x86/include/asm/processor.h > +++ b/arch/x86/include/asm/processor.h > @@ -530,6 +530,9 @@ struct thread_struct { > */ > u32 pkru; > > + unsigned long features; > + unsigned long features_locked; Should these be wrapped in #ifdef CONFIG_X86_SHADOW_STACK (or CONFIG_X86_CET) ? Also, just named "features"? Is this expected to be more than CET? > + > /* Floating point and extended processor state */ > struct fpu fpu; > /* > diff --git a/arch/x86/include/uapi/asm/prctl.h b/arch/x86/include/uapi/asm/prctl.h > index 500b96e71f18..028158e35269 100644 > --- a/arch/x86/include/uapi/asm/prctl.h > +++ b/arch/x86/include/uapi/asm/prctl.h > @@ -20,4 +20,10 @@ > #define ARCH_MAP_VDSO_32 0x2002 > #define ARCH_MAP_VDSO_64 0x2003 > > +/* Don't use 0x3001-0x3004 because of old glibcs */ > + > +#define ARCH_CET_ENABLE 0x4001 > +#define ARCH_CET_DISABLE 0x4002 > +#define ARCH_CET_LOCK 0x4003 > + > #endif /* _ASM_X86_PRCTL_H */ > diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c > index 58a6ea472db9..034880311e6b 100644 > --- a/arch/x86/kernel/process.c > +++ b/arch/x86/kernel/process.c > @@ -367,6 +367,10 @@ void arch_setup_new_exec(void) > task_clear_spec_ssb_noexec(current); > speculation_ctrl_update(read_thread_flags()); > } > + > + /* Reset thread features on exec */ > + current->thread.features = 0; > + current->thread.features_locked = 0; Same ifdef question here. > } > > #ifdef CONFIG_X86_IOPL_IOPERM > diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c > index 1962008fe743..8fa2c2b7de65 100644 > --- a/arch/x86/kernel/process_64.c > +++ b/arch/x86/kernel/process_64.c > @@ -829,7 +829,10 @@ long do_arch_prctl_64(struct task_struct *task, int option, unsigned long arg2) > case ARCH_MAP_VDSO_64: > return prctl_map_vdso(&vdso_image_64, arg2); > #endif > - > + case ARCH_CET_ENABLE: > + case ARCH_CET_DISABLE: > + case ARCH_CET_LOCK: > + return cet_prctl(task, option, arg2); > default: > ret = -EINVAL; > break; I remain annoyed that prctl interfaces didn't use -ENOTSUP for "unknown option". :P > diff --git a/arch/x86/kernel/shstk.c b/arch/x86/kernel/shstk.c > new file mode 100644 > index 000000000000..e3276ac9e9b9 > --- /dev/null > +++ b/arch/x86/kernel/shstk.c I think the Makefile addition should be moved from "x86/cet/shstk: Add user-mode shadow stack support" to here, yes? Otherwise, there is a bisectability randconfig-with-CONFIG_X86_SHADOW_STACK risk here (nothing will implement "cet_prctl"). > @@ -0,0 +1,38 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * shstk.c - Intel shadow stack support > + * > + * Copyright (c) 2021, Intel Corporation. > + * Yu-cheng Yu > + */ > + > +#include > +#include > +#include > + > +long cet_prctl(struct task_struct *task, int option, unsigned long features) > +{ > + if (option == ARCH_CET_LOCK) { > + task->thread.features_locked |= features; > + return 0; > + } > + > + /* Don't allow via ptrace */ > + if (task != current) > + return -EINVAL; ... but locking _is_ allowed via ptrace? If that intended, it should be explicitly mentioned in the commit log and in a comment here. Also, perhaps -ESRCH ? > + > + /* Do not allow to change locked features */ > + if (features & task->thread.features_locked) > + return -EPERM; > + > + /* Only support enabling/disabling one feature at a time. */ > + if (hweight_long(features) > 1) > + return -EINVAL; Perhaps -E2BIG ? > + if (option == ARCH_CET_DISABLE) { > + return -EINVAL; > + } > + > + /* Handle ARCH_CET_ENABLE */ > + return -EINVAL; > +} > -- > 2.17.1 > -- Kees Cook