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 46D88C4167B for ; Sat, 9 Dec 2023 03:15:30 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 5937B6B007E; Fri, 8 Dec 2023 22:15:29 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 543176B0080; Fri, 8 Dec 2023 22:15:29 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 3E3576B0081; Fri, 8 Dec 2023 22:15:29 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id 288F46B007E for ; Fri, 8 Dec 2023 22:15:29 -0500 (EST) Received: from smtpin04.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id DE3C11403BD for ; Sat, 9 Dec 2023 03:15:28 +0000 (UTC) X-FDA: 81545814336.04.43D6D3E Received: from mail-pg1-f171.google.com (mail-pg1-f171.google.com [209.85.215.171]) by imf07.hostedemail.com (Postfix) with ESMTP id F2FC840014 for ; Sat, 9 Dec 2023 03:15:26 +0000 (UTC) Authentication-Results: imf07.hostedemail.com; dkim=pass header.d=linaro.org header.s=google header.b=fK9Z41Pf; dmarc=pass (policy=none) header.from=linaro.org; spf=pass (imf07.hostedemail.com: domain of thiago.bauermann@linaro.org designates 209.85.215.171 as permitted sender) smtp.mailfrom=thiago.bauermann@linaro.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1702091727; 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=2b+6n+ULng3a579Szt1kcCIxHSrZ+D7LmH/Z+vqyyTE=; b=7WrXP11f7yub8gJ/9kbyw6Ic7+7yrTmJiktQ7fqgIjf69iJGUGhrx1B5bdWGKzDRMk0N3k cv+LU4/WDABSLCjNyMQFcOXPwWXTuzP15ywLk9tYlmRZsaBA96HHo/rziFY9I7T5XrVKV7 HJWVVF8UngsKzHqBP0qKqbTIuIyFzRM= ARC-Authentication-Results: i=1; imf07.hostedemail.com; dkim=pass header.d=linaro.org header.s=google header.b=fK9Z41Pf; dmarc=pass (policy=none) header.from=linaro.org; spf=pass (imf07.hostedemail.com: domain of thiago.bauermann@linaro.org designates 209.85.215.171 as permitted sender) smtp.mailfrom=thiago.bauermann@linaro.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1702091727; a=rsa-sha256; cv=none; b=vfa5QRUW69Vu/v62RP6VE4bJ2MYxbwE6w9HuNGUohDsHd2I69JVNJlVcQ/2buhlaZAlzr4 z39vJwz2vmzFzVLKAl2mfpER7HCMYdZMt8WjaudSPJB/EYkXw+C5QqCUJReBvaw8aQEmQ9 3strC0dHW+cVj7oHuA1RhrWlVuBPTg8= Received: by mail-pg1-f171.google.com with SMTP id 41be03b00d2f7-5c690c3d113so2239458a12.1 for ; Fri, 08 Dec 2023 19:15:26 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1702091726; x=1702696526; darn=kvack.org; h=mime-version:message-id:date:in-reply-to:subject:cc:to:from :user-agent:references:from:to:cc:subject:date:message-id:reply-to; bh=2b+6n+ULng3a579Szt1kcCIxHSrZ+D7LmH/Z+vqyyTE=; b=fK9Z41PfWag4h+OvYWuydzuLAutwu4W4zpN8LM9KaxtSFdIrJRoq87FvTLgAb2mPM5 /7lBdNsVeoV9OC0BBd8qMD0ePai9iiPLls/2uvr3muSkvp9JJWyIKjwTMdiMOA/c3eJa mNWn9MMprFpiE/mrhzUlbbMGRzwi7P3MjfrL6ViGObh87eJwmOrprLqy5xzUYwiW4YWl wwI8Wynfhx/9ntdepvD8Ns7wutMxqK5UN2cn4Ox1G3YPSJnYtzp+4/+CUMWMRpEiL/l+ IzzIiizi2q47QIkYFoUBJ2AtcwUoiE/9PZ2dauNYP9PWzUeYNybwcVzgJOy64Cbu6i/q TWSw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1702091726; x=1702696526; h=mime-version:message-id:date:in-reply-to:subject:cc:to:from :user-agent:references:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=2b+6n+ULng3a579Szt1kcCIxHSrZ+D7LmH/Z+vqyyTE=; b=o/iaZOXeb896h1otSYwm5nkHuvnLH+vF+rKvm++3cW3hZKCFnZ4f5DKcFfllfemra7 60bHB4DVK0oMHJcJhowrsCnV2TIMXKNqWnYyrfRa51K4zjqCkptfq+0qBZMZ8s+F2yjk WprvrHHku2nnDc0yujfgC5uTa+ZdBSIHLeFtZVogWCqtJTuC8e+d+Qhs6LYyf7uRWFD9 J2Wm6eFCzNqWpJ4oTRg5s/3cl1o+ONg/CsPWhHEqRtXioxkfdnWgJ8ofyX1SzG4hzdJi abCrqHORzRlxbqYzHsxyt45wEUQC7A3oS9VXrROOKp1wjBYuqlqekdQNZi7Ci1vndpaV WJHg== X-Gm-Message-State: AOJu0Yznj0Oj0nqH0aeem/yLULxFrEknkF4q6DB3KnD1GAmJP2Pb0ka+ 9BjApJDllQI3Q9q7M0gQ5W6KEA== X-Google-Smtp-Source: AGHT+IEaUnkmb6VMOC1ZmV7RhknH3M28z+3B6TqVq+yqgnnJz1v02mq63uTBtrSHn4oV4teABtH6uA== X-Received: by 2002:a05:6a20:f390:b0:190:6920:e14b with SMTP id qr16-20020a056a20f39000b001906920e14bmr1018200pzb.122.1702091725612; Fri, 08 Dec 2023 19:15:25 -0800 (PST) Received: from localhost ([2804:14d:7e39:8470:4c58:a216:27d2:2ff]) by smtp.gmail.com with ESMTPSA id x22-20020a056a00271600b006be5af77f06sm2330693pfv.2.2023.12.08.19.15.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 08 Dec 2023 19:15:25 -0800 (PST) References: <20231122-arm64-gcs-v7-0-201c483bd775@kernel.org> <20231122-arm64-gcs-v7-24-201c483bd775@kernel.org> User-agent: mu4e 1.10.8; emacs 29.1 From: Thiago Jung Bauermann To: Mark Brown Cc: Catalin Marinas , Will Deacon , Jonathan Corbet , Andrew Morton , Marc Zyngier , Oliver Upton , James Morse , Suzuki K Poulose , Arnd Bergmann , Oleg Nesterov , Eric Biederman , Kees Cook , Shuah Khan , "Rick P. Edgecombe" , Deepak Gupta , Ard Biesheuvel , Szabolcs Nagy , "H.J. Lu" , Paul Walmsley , Palmer Dabbelt , Albert Ou , Florian Weimer , Christian Brauner , linux-arm-kernel@lists.infradead.org, linux-doc@vger.kernel.org, kvmarm@lists.linux.dev, linux-fsdevel@vger.kernel.org, linux-arch@vger.kernel.org, linux-mm@kvack.org, linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org, linux-riscv@lists.infradead.org Subject: Re: [PATCH v7 24/39] arm64/signal: Set up and restore the GCS context for signal handlers In-reply-to: <20231122-arm64-gcs-v7-24-201c483bd775@kernel.org> Date: Sat, 09 Dec 2023 00:15:22 -0300 Message-ID: <8734wcgj79.fsf@linaro.org> MIME-Version: 1.0 Content-Type: text/plain X-Rspamd-Queue-Id: F2FC840014 X-Rspam-User: X-Rspamd-Server: rspam02 X-Stat-Signature: d1pe5z8zixuaexh4qwbzfa718wp785td X-HE-Tag: 1702091726-286311 X-HE-Meta: U2FsdGVkX18FyrUhmBWqPpiXQUDz+6/svZMqjTNLTaGFdPOXYYteu5wBevgFrl8t4Sg1Pr4FnJyKuRO7GQI61a//1S7E7WQua2INhFWSf1j5XV+/L0bvitlF7sET+G9lJprFOPM8f3UTdhaQoPATztRARRd2rq/lEyul/PGhiDy943YYwi2n2T5NzAlbnTrhDnWG236MeoMavhsZEa7D0g9aRO6dW3lGoqGsH2MdbwOS2ojnIEwG06NwND0U246tPhA2U4s1qb9SMvUzNGvH3ttDMfhJCsWn3JYY1Z40n/eov8OWvOjDZheOumiuB8Fhw7taBaYXeAl5US0mQdDl4rIi+tUt7sGuw4PvF1iwFZ0gIUIlVXsfGGOGrk03aBx7mFSJf2VwgugrVtTfRmCaycqzb9WvM+45aGej5kX0d5eLPxGYBDV9y5CEBKg6TRNLNSqXotWLquhYmE00IVMvPUcw1uGswG21Cj4kY4ejkyOMwCbbzPyONoEDgx6baJ/CjBNu+3utqGekB3vGcqI3zYdPOn7MuGdB2elruyiy4cih08jXDc+RJ6H05ss8NC5wd5Q4cyI8Qz1JVElId0pYu7wxYpNI947b9A7q4n4pROgzWkuL3exQUF4wlxbSNj2MswLzA1hxnkWRqdkSaOKOVUc3HjK/QoZqft9vTiqc6R98NLTd2A5B9WX5K5uJy1mRwYwnT8Rt8HFELc0tER/oLlTqsk8scx83EcTnd2osym9r+0c5rXbx37eztPBXcdO3NqEyePK/duQ2hIwl6scrtJyniX0op40YBPyjd5Im4FTMYzlzZqcjmjNGZZyMmP0D2evBE8wWTgAwgZmEexj4klfUjaJ66nQ/gPQKEzHMER5cJ/GF2ja675/oS71DSUdktYuJYCFpyaNsZjm+vmd3GhBwGSYQIh+mg+sM65uYurhmsfLSi9JSR47KaMp/rHM9TTgQ6mmeg3GO6mc8Gl9 ohOcsnb9 6tVKyFs5/c0Vw3uDjD8OJa0dX5qdKOfqx7nyud/xeCVQgmTc8bdhHTpXEpyiFWjN8j/zLNDrtfhbAhy1deYyV4IhsSmmxWfqvAsjCljGXdu4VZ/Kc8IFvaAzOrKTd0+udvkSfNSH0FaU/oj0Pe9rfYEt1VI6+Qn4s9PlBgQ6D+dlFiWvXQ2+uAKORcd5Xqe2y7VizZpH2Fd/tMgpkZE4WG5z08qbt1OvV589dynbVhvpsnHtpep+hNpj6W3SlemGn4P9UVdC0BSRexCep/TlHYpBLUBSH1q+e3KYU/jwkAFb6HhuPtmW93Z7xFadXqjMbYldUSzKFz2BzCeJVScF17WN48MNThW5iNupvukD/iN/7hlEa9z/mP0Ds6MT7USf6NWrtOkB8VL20p2WIUIbhUZlfGkPGVkzZ/7ppeGeO+kP2e3dyEZxfS8JbYP8vE5YjYJ4K7NV2cJz0Maoo0EHUuciDVDYgOQZ8JXsc 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: List-Subscribe: List-Unsubscribe: Mark Brown writes: > +static bool gcs_signal_cap_valid(u64 addr, u64 val) > +{ > + /* > + * The top bit should be set, this is an invalid address for > + * EL0 and will only be set for caps created by signals. > + */ > + if (!(val & GCS_SIGNAL_CAP_FLAG)) > + return false; > + > + /* The rest should be a standard architectural cap token. */ > + val &= ~GCS_SIGNAL_CAP_FLAG; > + > + /* The cap must have the low bits set to a token value */ > + if (GCS_CAP_TOKEN(val) != 0) > + return false; I found the comment above a little confusing, since the if condition actually checks that low bits aren't set at all. Perhaps reword to something like "The token value of a signal cap must be 0"? > + > + /* The cap must store the VA the cap was stored at */ > + if (GCS_CAP_ADDR(addr) != GCS_CAP_ADDR(val)) > + return false; > + > + return true; > +} > +#endif > + > /* > * Do a signal return; undo the signal stack. These are aligned to 128-bit. > */ > @@ -815,6 +847,45 @@ static int restore_sigframe(struct pt_regs *regs, > return err; > } > > +#ifdef CONFIG_ARM64_GCS > +static int gcs_restore_signal(void) > +{ > + u64 gcspr_el0, cap; > + int ret; > + > + if (!system_supports_gcs()) > + return 0; > + > + if (!(current->thread.gcs_el0_mode & PR_SHADOW_STACK_ENABLE)) > + return 0; > + > + gcspr_el0 = read_sysreg_s(SYS_GCSPR_EL0); > + > + /* > + * GCSPR_EL0 should be pointing at a capped GCS, read the cap... > + */ > + gcsb_dsync(); > + ret = copy_from_user(&cap, (__user void*)gcspr_el0, sizeof(cap)); > + if (ret) > + return -EFAULT; > + > + /* > + * ...then check that the cap is the actual GCS before > + * restoring it. > + */ > + if (!gcs_signal_cap_valid(gcspr_el0, cap)) > + return -EINVAL; > + > + current->thread.gcspr_el0 = gcspr_el0 + sizeof(cap); > + write_sysreg_s(current->thread.gcspr_el0, SYS_GCSPR_EL0); At this point, there's an inactive but valid cap just below the GCS. Over time, as different signals are received when the GCSPR is pointing at different locations of the stack, there could be a number of valid inactive caps available for misuse. I'm still not proficient enough in GCS to know how exactly this could be abused (e.g., somehow writing the desired return location right above one of these inactive caps and arranging for GCSPR to point to the cap before returning from a signal) but to be safe or paranoid, perhaps zero the location of the cap before returning? > + > + return 0; > +} > + > +#else > +static int gcs_restore_signal(void) { return 0; } > +#endif > + > SYSCALL_DEFINE0(rt_sigreturn) > { > struct pt_regs *regs = current_pt_regs(); > @@ -841,6 +912,9 @@ SYSCALL_DEFINE0(rt_sigreturn) > if (restore_altstack(&frame->uc.uc_stack)) > goto badframe; > > + if (gcs_restore_signal()) > + goto badframe; > + > return regs->regs[0]; > > badframe: > @@ -1071,7 +1145,50 @@ static int get_sigframe(struct rt_sigframe_user_layout *user, > return 0; > } > > -static void setup_return(struct pt_regs *regs, struct k_sigaction *ka, > +#ifdef CONFIG_ARM64_GCS > + > +static int gcs_signal_entry(__sigrestore_t sigtramp, struct ksignal *ksig) The ksig argument is unused, so it can be removed. > +{ > + unsigned long __user *gcspr_el0; > + int ret = 0; > + > + if (!system_supports_gcs()) > + return 0; > + > + if (!task_gcs_el0_enabled(current)) > + return 0; > + > + /* > + * We are entering a signal handler, current register state is > + * active. > + */ > + gcspr_el0 = (unsigned long __user *)read_sysreg_s(SYS_GCSPR_EL0); > + > + /* > + * Push a cap and the GCS entry for the trampoline onto the GCS. > + */ > + put_user_gcs((unsigned long)sigtramp, gcspr_el0 - 2, &ret); > + put_user_gcs(GCS_SIGNAL_CAP(gcspr_el0 - 1), gcspr_el0 - 1, &ret); > + if (ret != 0) > + return ret; > + > + gcsb_dsync(); > + > + gcspr_el0 -= 2; > + write_sysreg_s((unsigned long)gcspr_el0, SYS_GCSPR_EL0); > + > + return 0; > +} > +#else > + > +static int gcs_signal_entry(__sigrestore_t sigtramp, struct ksignal *ksig) > +{ > + return 0; > +} > + > +#endif > + > +static int setup_return(struct pt_regs *regs, struct ksignal *ksig, > struct rt_sigframe_user_layout *user, int usig) Since the ksig argument isn't used by gcs_signal_entry(), setup_return() can keep the ka argument and the changes below from ka to ksic->ka are unnecessary. > { > __sigrestore_t sigtramp; > @@ -1079,7 +1196,7 @@ static void setup_return(struct pt_regs *regs, struct k_sigaction *ka, > regs->regs[0] = usig; > regs->sp = (unsigned long)user->sigframe; > regs->regs[29] = (unsigned long)&user->next_frame->fp; > - regs->pc = (unsigned long)ka->sa.sa_handler; > + regs->pc = (unsigned long)ksig->ka.sa.sa_handler; > > /* > * Signal delivery is a (wacky) indirect function call in > @@ -1119,12 +1236,14 @@ static void setup_return(struct pt_regs *regs, struct k_sigaction *ka, > sme_smstop(); > } > > - if (ka->sa.sa_flags & SA_RESTORER) > - sigtramp = ka->sa.sa_restorer; > + if (ksig->ka.sa.sa_flags & SA_RESTORER) > + sigtramp = ksig->ka.sa.sa_restorer; > else > sigtramp = VDSO_SYMBOL(current->mm->context.vdso, sigtramp); > > regs->regs[30] = (unsigned long)sigtramp; > + > + return gcs_signal_entry(sigtramp, ksig); > } > > static int setup_rt_frame(int usig, struct ksignal *ksig, sigset_t *set, > @@ -1147,7 +1266,7 @@ static int setup_rt_frame(int usig, struct ksignal *ksig, sigset_t *set, > err |= __save_altstack(&frame->uc.uc_stack, regs->sp); > err |= setup_sigframe(&user, regs, set); > if (err == 0) { > - setup_return(regs, &ksig->ka, &user, usig); > + err = setup_return(regs, ksig, &user, usig); > if (ksig->ka.sa.sa_flags & SA_SIGINFO) { > err |= copy_siginfo_to_user(&frame->info, &ksig->info); > regs->regs[1] = (unsigned long)&frame->info; > diff --git a/arch/arm64/mm/gcs.c b/arch/arm64/mm/gcs.c > index 02f8f6046c10..6f51429c5a46 100644 > --- a/arch/arm64/mm/gcs.c > +++ b/arch/arm64/mm/gcs.c > @@ -6,6 +6,7 @@ > #include > > #include > +#include > #include This is #include isn't needed by this patch. Probably better as part of another one. -- Thiago