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 X-Spam-Level: X-Spam-Status: No, score=-18.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_IN_DEF_DKIM_WL autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 91553C4320A for ; Thu, 19 Aug 2021 23:48:42 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 7B38C610E5 for ; Thu, 19 Aug 2021 23:48:42 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236585AbhHSXtS (ORCPT ); Thu, 19 Aug 2021 19:49:18 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:60328 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236809AbhHSXtN (ORCPT ); Thu, 19 Aug 2021 19:49:13 -0400 Received: from mail-pj1-x1035.google.com (mail-pj1-x1035.google.com [IPv6:2607:f8b0:4864:20::1035]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 13CF3C0613CF for ; Thu, 19 Aug 2021 16:48:37 -0700 (PDT) Received: by mail-pj1-x1035.google.com with SMTP id hv22-20020a17090ae416b0290178c579e424so5994376pjb.3 for ; Thu, 19 Aug 2021 16:48:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=peyh3WLA8DgwuaakoXbly04TuP2J4HXY0owEUug9y1c=; b=BbFJLtvxbxBG+7picsXObo5Qj8iquJ+C6Q7w5pe27YupZ7+zDXLN540+QU8HPzkPh2 WwzvQTknLR7mJUL2ZtsslkF86UhNT3PUimzJd1W/xc0YsfWgxj0uNN/X65pRaldPuQy1 Svz/l9I2+rXfrKEmbGry3ew82Uw0db0pKDxq9ukmqwtSMFXcHcAuMrcqlCqOeGH1V/uC AqRS5aTXg3eyxy84gMMrOJhW6Ac+BzpVGDyg6q+eRN+OlPt5iJkRzEHzotLklvgBWCCO ThS3AjXFs8AEz4pcXFmbRPSyBU/T0LhtGGlVC0DabZEeUTxAm2SIBNQHSE15qMVZvLr/ ceIA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=peyh3WLA8DgwuaakoXbly04TuP2J4HXY0owEUug9y1c=; b=XRmo+nQJ4OQ3rYN69oTfq840YgWKhvWEPr9zVnU6+vvD/IAAblUw+FKK2TKkIusYii PHk99DjUU2e//WAQmYNI4c7EoNzj6GapC91r67NfxamyZn6cgUrrY4nDJZ2eanZbjXEo bp5ZESDSGY6wt00Tjm2zTh2IzR3IJ6Z9euccsITEEJVuwWb8Rd+umcU3P1XY1cP11L+C xu3VCCpF9qksnVTOcwbT9KNZzUAHUowPSgpMr/wrbjWN2yLyfUPhlnkBB85yEqOHHzfw uWKel6hXPpLcGGT3AXn8d95WHv0DlgqtTvNKnXCspwHVQTZA31c385Z/msd39j7WRA5p Ql4Q== X-Gm-Message-State: AOAM5301rmxOClhHUhJk/gSxozGJzdMi57+tfitnLg+tMTawqeAr2nei NtWY57d7MZIuR3P2HiGRXVToiQ== X-Google-Smtp-Source: ABdhPJwX9PVK0tEzKb91C/ktX1nj3RcgHWNnmrdk1FjTlt3T+0IB78U5SgxuhlVnRMNmyLPH0lN5vw== X-Received: by 2002:a17:90b:80c:: with SMTP id bk12mr1415764pjb.134.1629416916083; Thu, 19 Aug 2021 16:48:36 -0700 (PDT) Received: from google.com (157.214.185.35.bc.googleusercontent.com. [35.185.214.157]) by smtp.gmail.com with ESMTPSA id a4sm4880315pfk.0.2021.08.19.16.48.35 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 19 Aug 2021 16:48:35 -0700 (PDT) Date: Thu, 19 Aug 2021 23:48:29 +0000 From: Sean Christopherson To: Mathieu Desnoyers Cc: "Russell King, ARM Linux" , Catalin Marinas , Will Deacon , Guo Ren , Thomas Bogendoerfer , Michael Ellerman , Heiko Carstens , gor , Christian Borntraeger , Oleg Nesterov , rostedt , Ingo Molnar , Thomas Gleixner , Peter Zijlstra , Andy Lutomirski , paulmck , Boqun Feng , Paolo Bonzini , shuah , Benjamin Herrenschmidt , Paul Mackerras , linux-arm-kernel , linux-kernel , linux-csky , linux-mips@vger.kernel.org, linuxppc-dev , linux-s390@vger.kernel.org, KVM list , linux-kselftest , Peter Foley , Shakeel Butt , Ben Gardon Subject: Re: [PATCH 1/5] KVM: rseq: Update rseq when processing NOTIFY_RESUME on xfer to KVM guest Message-ID: References: <20210818001210.4073390-1-seanjc@google.com> <20210818001210.4073390-2-seanjc@google.com> <1673583543.19718.1629409152244.JavaMail.zimbra@efficios.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1673583543.19718.1629409152244.JavaMail.zimbra@efficios.com> Precedence: bulk List-ID: X-Mailing-List: linux-csky@vger.kernel.org On Thu, Aug 19, 2021, Mathieu Desnoyers wrote: > ----- On Aug 17, 2021, at 8:12 PM, Sean Christopherson seanjc@google.com wrote: > > @@ -250,7 +250,7 @@ static int rseq_ip_fixup(struct pt_regs *regs) > > * If not nested over a rseq critical section, restart is useless. > > * Clear the rseq_cs pointer and return. > > */ > > - if (!in_rseq_cs(ip, &rseq_cs)) > > + if (!regs || !in_rseq_cs(ip, &rseq_cs)) > > I think clearing the thread's rseq_cs unconditionally here when regs is NULL > is not the behavior we want when this is called from xfer_to_guest_mode_work. > > If we have a scenario where userspace ends up calling this ioctl(KVM_RUN) > from within a rseq c.s., we really want a CONFIG_DEBUG_RSEQ=y kernel to > kill this application in the rseq_syscall handler when exiting back to usermode > when the ioctl eventually returns. > > However, clearing the thread's rseq_cs will prevent this from happening. > > So I would favor an approach where we simply do: > > if (!regs) > return 0; > > Immediately at the beginning of rseq_ip_fixup, before getting the instruction > pointer, so effectively skip all side-effects of the ip fixup code. Indeed, it > is not relevant to do any fixup here, because it is nested in a ioctl system > call. > > Effectively, this would preserve the SIGSEGV behavior when this ioctl is > erroneously called by user-space from a rseq critical section. Ha, that's effectively what I implemented first, but I changed it because of the comment in clear_rseq_cs() that says: The rseq_cs field is set to NULL on preemption or signal delivery ... as well as well as on top of code outside of the rseq assembly block. Which makes it sound like something might rely on clearing rseq_cs? Ah, or is it the case that rseq_cs is non-NULL if and only if userspace is in an rseq critical section, and because syscalls in critical sections are illegal, by definition clearing rseq_cs is a nop unless userspace is misbehaving. If that's true, what about explicitly checking that at NOTIFY_RESUME? Or is it not worth the extra code to detect an error that will likely be caught anyways? diff --git a/kernel/rseq.c b/kernel/rseq.c index 35f7bd0fced0..28b8342290b0 100644 --- a/kernel/rseq.c +++ b/kernel/rseq.c @@ -282,6 +282,13 @@ void __rseq_handle_notify_resume(struct ksignal *ksig, struct pt_regs *regs) if (unlikely(t->flags & PF_EXITING)) return; + if (!regs) { +#ifdef CONFIG_DEBUG_RSEQ + if (t->rseq && rseq_get_rseq_cs(t, &rseq_cs)) + goto error; +#endif + return; + } ret = rseq_ip_fixup(regs); if (unlikely(ret < 0)) goto error; > Thanks for looking into this ! > > Mathieu > > > return clear_rseq_cs(t); > > ret = rseq_need_restart(t, rseq_cs.flags); > > if (ret <= 0) > > -- > > 2.33.0.rc1.237.g0d66db33f3-goog > > -- > Mathieu Desnoyers > EfficiOS Inc. > http://www.efficios.com 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 X-Spam-Level: X-Spam-Status: No, score=-8.6 required=3.0 tests=BAYES_00,DKIM_ADSP_CUSTOM_MED, DKIM_INVALID,DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id EB63BC4338F for ; Thu, 19 Aug 2021 23:49:24 +0000 (UTC) Received: from lists.ozlabs.org (lists.ozlabs.org [112.213.38.117]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 0CCCB610E5 for ; Thu, 19 Aug 2021 23:49:23 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 0CCCB610E5 Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=lists.ozlabs.org Received: from boromir.ozlabs.org (localhost [IPv6:::1]) by lists.ozlabs.org (Postfix) with ESMTP id 4GrM1Z2tcsz3cX6 for ; Fri, 20 Aug 2021 09:49:22 +1000 (AEST) Authentication-Results: lists.ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=google.com header.i=@google.com header.a=rsa-sha256 header.s=20161025 header.b=BbFJLtvx; dkim-atps=neutral Authentication-Results: lists.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=google.com (client-ip=2607:f8b0:4864:20::102e; helo=mail-pj1-x102e.google.com; envelope-from=seanjc@google.com; receiver=) Authentication-Results: lists.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=google.com header.i=@google.com header.a=rsa-sha256 header.s=20161025 header.b=BbFJLtvx; dkim-atps=neutral Received: from mail-pj1-x102e.google.com (mail-pj1-x102e.google.com [IPv6:2607:f8b0:4864:20::102e]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 4GrM0n503qz2xlC for ; Fri, 20 Aug 2021 09:48:39 +1000 (AEST) Received: by mail-pj1-x102e.google.com with SMTP id n5so6117777pjt.4 for ; Thu, 19 Aug 2021 16:48:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=peyh3WLA8DgwuaakoXbly04TuP2J4HXY0owEUug9y1c=; b=BbFJLtvxbxBG+7picsXObo5Qj8iquJ+C6Q7w5pe27YupZ7+zDXLN540+QU8HPzkPh2 WwzvQTknLR7mJUL2ZtsslkF86UhNT3PUimzJd1W/xc0YsfWgxj0uNN/X65pRaldPuQy1 Svz/l9I2+rXfrKEmbGry3ew82Uw0db0pKDxq9ukmqwtSMFXcHcAuMrcqlCqOeGH1V/uC AqRS5aTXg3eyxy84gMMrOJhW6Ac+BzpVGDyg6q+eRN+OlPt5iJkRzEHzotLklvgBWCCO ThS3AjXFs8AEz4pcXFmbRPSyBU/T0LhtGGlVC0DabZEeUTxAm2SIBNQHSE15qMVZvLr/ ceIA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=peyh3WLA8DgwuaakoXbly04TuP2J4HXY0owEUug9y1c=; b=YL/SsOqYrZwsMMoTwwSWUGYj+Sl+nPiJFaBtezzISjgt4bVP1N3soz17/iyxmT/anx yNUcJRaRlIjXL9N9t+D+J+EGHho6o8kqJK3Tw+bY/2u993oi5/v8qSLt6NiD5LD4AnUy s2Nx/aS5jLhalmeLUJsKLrN8wTdTS5mqShrc2gznURMkkg8UVnHPvG+Rj8LOb5WgJZ4L gXndi2JZiLen8ralrIQnHuxst5h1+E1ntCTEFp/lIcur+gv6o7QE8yS0Pw+HJGGfYlSO YRFzUO+MN1PLAw+k6bJVVbkpxq944NK6RbwV3WI+gi3zjfNbbbvqFGkA5pef6Z2HTAVL 9wJQ== X-Gm-Message-State: AOAM530x+Qu1f+vEs7pC5PzOTHiLD63pgt8bQSi5TxTbpLwYphjuzjY1 07lc42fj3auj1/NUz1BLdTYmhA== X-Google-Smtp-Source: ABdhPJwX9PVK0tEzKb91C/ktX1nj3RcgHWNnmrdk1FjTlt3T+0IB78U5SgxuhlVnRMNmyLPH0lN5vw== X-Received: by 2002:a17:90b:80c:: with SMTP id bk12mr1415764pjb.134.1629416916083; Thu, 19 Aug 2021 16:48:36 -0700 (PDT) Received: from google.com (157.214.185.35.bc.googleusercontent.com. [35.185.214.157]) by smtp.gmail.com with ESMTPSA id a4sm4880315pfk.0.2021.08.19.16.48.35 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 19 Aug 2021 16:48:35 -0700 (PDT) Date: Thu, 19 Aug 2021 23:48:29 +0000 From: Sean Christopherson To: Mathieu Desnoyers Subject: Re: [PATCH 1/5] KVM: rseq: Update rseq when processing NOTIFY_RESUME on xfer to KVM guest Message-ID: References: <20210818001210.4073390-1-seanjc@google.com> <20210818001210.4073390-2-seanjc@google.com> <1673583543.19718.1629409152244.JavaMail.zimbra@efficios.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1673583543.19718.1629409152244.JavaMail.zimbra@efficios.com> X-BeenThere: linuxppc-dev@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: KVM list , Peter Zijlstra , linux-kernel , Will Deacon , Guo Ren , linux-kselftest , Ben Gardon , shuah , Paul Mackerras , linux-s390@vger.kernel.org, gor , "Russell King, ARM Linux" , linux-csky , Christian Borntraeger , Ingo Molnar , Catalin Marinas , linux-mips@vger.kernel.org, Boqun Feng , paulmck , Heiko Carstens , rostedt , Shakeel Butt , Andy Lutomirski , Thomas Gleixner , Peter Foley , linux-arm-kernel , Thomas Bogendoerfer , Oleg Nesterov , Paolo Bonzini , linuxppc-dev Errors-To: linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Sender: "Linuxppc-dev" On Thu, Aug 19, 2021, Mathieu Desnoyers wrote: > ----- On Aug 17, 2021, at 8:12 PM, Sean Christopherson seanjc@google.com wrote: > > @@ -250,7 +250,7 @@ static int rseq_ip_fixup(struct pt_regs *regs) > > * If not nested over a rseq critical section, restart is useless. > > * Clear the rseq_cs pointer and return. > > */ > > - if (!in_rseq_cs(ip, &rseq_cs)) > > + if (!regs || !in_rseq_cs(ip, &rseq_cs)) > > I think clearing the thread's rseq_cs unconditionally here when regs is NULL > is not the behavior we want when this is called from xfer_to_guest_mode_work. > > If we have a scenario where userspace ends up calling this ioctl(KVM_RUN) > from within a rseq c.s., we really want a CONFIG_DEBUG_RSEQ=y kernel to > kill this application in the rseq_syscall handler when exiting back to usermode > when the ioctl eventually returns. > > However, clearing the thread's rseq_cs will prevent this from happening. > > So I would favor an approach where we simply do: > > if (!regs) > return 0; > > Immediately at the beginning of rseq_ip_fixup, before getting the instruction > pointer, so effectively skip all side-effects of the ip fixup code. Indeed, it > is not relevant to do any fixup here, because it is nested in a ioctl system > call. > > Effectively, this would preserve the SIGSEGV behavior when this ioctl is > erroneously called by user-space from a rseq critical section. Ha, that's effectively what I implemented first, but I changed it because of the comment in clear_rseq_cs() that says: The rseq_cs field is set to NULL on preemption or signal delivery ... as well as well as on top of code outside of the rseq assembly block. Which makes it sound like something might rely on clearing rseq_cs? Ah, or is it the case that rseq_cs is non-NULL if and only if userspace is in an rseq critical section, and because syscalls in critical sections are illegal, by definition clearing rseq_cs is a nop unless userspace is misbehaving. If that's true, what about explicitly checking that at NOTIFY_RESUME? Or is it not worth the extra code to detect an error that will likely be caught anyways? diff --git a/kernel/rseq.c b/kernel/rseq.c index 35f7bd0fced0..28b8342290b0 100644 --- a/kernel/rseq.c +++ b/kernel/rseq.c @@ -282,6 +282,13 @@ void __rseq_handle_notify_resume(struct ksignal *ksig, struct pt_regs *regs) if (unlikely(t->flags & PF_EXITING)) return; + if (!regs) { +#ifdef CONFIG_DEBUG_RSEQ + if (t->rseq && rseq_get_rseq_cs(t, &rseq_cs)) + goto error; +#endif + return; + } ret = rseq_ip_fixup(regs); if (unlikely(ret < 0)) goto error; > Thanks for looking into this ! > > Mathieu > > > return clear_rseq_cs(t); > > ret = rseq_need_restart(t, rseq_cs.flags); > > if (ret <= 0) > > -- > > 2.33.0.rc1.237.g0d66db33f3-goog > > -- > Mathieu Desnoyers > EfficiOS Inc. > http://www.efficios.com 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 X-Spam-Level: X-Spam-Status: No, score=-9.4 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_ADSP_CUSTOM_MED,DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 51AE8C4338F for ; Thu, 19 Aug 2021 23:51:04 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 15DA560230 for ; Thu, 19 Aug 2021 23:51:04 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 15DA560230 Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=tvN1CJZQcHnLCOY29R3bSStUOktyZYwRKg3BZOX7SXQ=; b=IZ58QXbW6B+Qlb zQLgjDLxig78og/9Djed0ROlnaXBSlK684wnDk/74z1inGI6tV2ZMWfncAnFfDspQiRo+VCOWNB4M dNtyWwEINvcawqtZuz0wvCKBNNOfK8o2ntHDy/JrdNEHMb5fsphy6oXJRahiINzipkz16IFtsXPVi BHGW9L5rnho30OaNIr45AdXSi7a9RWdyBKbAjWeJq2KHfNHwmj7xSY2yzedUilLt/sd+I2/bk8gEO 5L0IUl4Hv50JPdNul2aNNbOEX+PCuDuLr9gR/Lz1tAJM8o7PcgvTQGTK+uKByAyGMEw1CO568RZ4g p6s+uu6WcwiSNNaI7zDQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mGrm7-009lnf-Qy; Thu, 19 Aug 2021 23:48:44 +0000 Received: from mail-pj1-x1029.google.com ([2607:f8b0:4864:20::1029]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1mGrm1-009lmh-4A for linux-arm-kernel@lists.infradead.org; Thu, 19 Aug 2021 23:48:41 +0000 Received: by mail-pj1-x1029.google.com with SMTP id j1so6122334pjv.3 for ; Thu, 19 Aug 2021 16:48:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=peyh3WLA8DgwuaakoXbly04TuP2J4HXY0owEUug9y1c=; b=BbFJLtvxbxBG+7picsXObo5Qj8iquJ+C6Q7w5pe27YupZ7+zDXLN540+QU8HPzkPh2 WwzvQTknLR7mJUL2ZtsslkF86UhNT3PUimzJd1W/xc0YsfWgxj0uNN/X65pRaldPuQy1 Svz/l9I2+rXfrKEmbGry3ew82Uw0db0pKDxq9ukmqwtSMFXcHcAuMrcqlCqOeGH1V/uC AqRS5aTXg3eyxy84gMMrOJhW6Ac+BzpVGDyg6q+eRN+OlPt5iJkRzEHzotLklvgBWCCO ThS3AjXFs8AEz4pcXFmbRPSyBU/T0LhtGGlVC0DabZEeUTxAm2SIBNQHSE15qMVZvLr/ ceIA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=peyh3WLA8DgwuaakoXbly04TuP2J4HXY0owEUug9y1c=; b=UzQiZgbTx7tafz1VM8SG5j60id2Venpt3DqINE42tYsrNIchEa4P2WT4N8p9NFOuu0 2ELHlP3CWDE0BFYbOvL7i7SeEwc79CrnoEOGG5T866vj9P4mrmhNVA87MVb5ed+N9mXA yPuSfwh14dc4Izh2RBaw9mtIp4SdvBGNkyfJPS9bCC1Qpu7bOjK4zInS4DH+LCwfOTlt eQmqYRR5DB5j0nsGfs77AO8WWUw2Q2271QydTwZlFflgHEO/eyJFk6ROCb+PyrSsXPhb pPVMAcqLTakKU559+Ddi9+j2kp6sL+W+xi3gfWuWnck3ev/TRS2/rMRl2O2ixGuYwqR9 ijQw== X-Gm-Message-State: AOAM533I5Lbq/zOmWpr2UNkHfiGRopGeappp/rT3Cj/eQdyRC0c0lk7V 9cJ61l7WnsQr0nlu6fPcZXoywg== X-Google-Smtp-Source: ABdhPJwX9PVK0tEzKb91C/ktX1nj3RcgHWNnmrdk1FjTlt3T+0IB78U5SgxuhlVnRMNmyLPH0lN5vw== X-Received: by 2002:a17:90b:80c:: with SMTP id bk12mr1415764pjb.134.1629416916083; Thu, 19 Aug 2021 16:48:36 -0700 (PDT) Received: from google.com (157.214.185.35.bc.googleusercontent.com. [35.185.214.157]) by smtp.gmail.com with ESMTPSA id a4sm4880315pfk.0.2021.08.19.16.48.35 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 19 Aug 2021 16:48:35 -0700 (PDT) Date: Thu, 19 Aug 2021 23:48:29 +0000 From: Sean Christopherson To: Mathieu Desnoyers Cc: "Russell King, ARM Linux" , Catalin Marinas , Will Deacon , Guo Ren , Thomas Bogendoerfer , Michael Ellerman , Heiko Carstens , gor , Christian Borntraeger , Oleg Nesterov , rostedt , Ingo Molnar , Thomas Gleixner , Peter Zijlstra , Andy Lutomirski , paulmck , Boqun Feng , Paolo Bonzini , shuah , Benjamin Herrenschmidt , Paul Mackerras , linux-arm-kernel , linux-kernel , linux-csky , linux-mips@vger.kernel.org, linuxppc-dev , linux-s390@vger.kernel.org, KVM list , linux-kselftest , Peter Foley , Shakeel Butt , Ben Gardon Subject: Re: [PATCH 1/5] KVM: rseq: Update rseq when processing NOTIFY_RESUME on xfer to KVM guest Message-ID: References: <20210818001210.4073390-1-seanjc@google.com> <20210818001210.4073390-2-seanjc@google.com> <1673583543.19718.1629409152244.JavaMail.zimbra@efficios.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1673583543.19718.1629409152244.JavaMail.zimbra@efficios.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210819_164837_237019_12773A5F X-CRM114-Status: GOOD ( 30.80 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Thu, Aug 19, 2021, Mathieu Desnoyers wrote: > ----- On Aug 17, 2021, at 8:12 PM, Sean Christopherson seanjc@google.com wrote: > > @@ -250,7 +250,7 @@ static int rseq_ip_fixup(struct pt_regs *regs) > > * If not nested over a rseq critical section, restart is useless. > > * Clear the rseq_cs pointer and return. > > */ > > - if (!in_rseq_cs(ip, &rseq_cs)) > > + if (!regs || !in_rseq_cs(ip, &rseq_cs)) > > I think clearing the thread's rseq_cs unconditionally here when regs is NULL > is not the behavior we want when this is called from xfer_to_guest_mode_work. > > If we have a scenario where userspace ends up calling this ioctl(KVM_RUN) > from within a rseq c.s., we really want a CONFIG_DEBUG_RSEQ=y kernel to > kill this application in the rseq_syscall handler when exiting back to usermode > when the ioctl eventually returns. > > However, clearing the thread's rseq_cs will prevent this from happening. > > So I would favor an approach where we simply do: > > if (!regs) > return 0; > > Immediately at the beginning of rseq_ip_fixup, before getting the instruction > pointer, so effectively skip all side-effects of the ip fixup code. Indeed, it > is not relevant to do any fixup here, because it is nested in a ioctl system > call. > > Effectively, this would preserve the SIGSEGV behavior when this ioctl is > erroneously called by user-space from a rseq critical section. Ha, that's effectively what I implemented first, but I changed it because of the comment in clear_rseq_cs() that says: The rseq_cs field is set to NULL on preemption or signal delivery ... as well as well as on top of code outside of the rseq assembly block. Which makes it sound like something might rely on clearing rseq_cs? Ah, or is it the case that rseq_cs is non-NULL if and only if userspace is in an rseq critical section, and because syscalls in critical sections are illegal, by definition clearing rseq_cs is a nop unless userspace is misbehaving. If that's true, what about explicitly checking that at NOTIFY_RESUME? Or is it not worth the extra code to detect an error that will likely be caught anyways? diff --git a/kernel/rseq.c b/kernel/rseq.c index 35f7bd0fced0..28b8342290b0 100644 --- a/kernel/rseq.c +++ b/kernel/rseq.c @@ -282,6 +282,13 @@ void __rseq_handle_notify_resume(struct ksignal *ksig, struct pt_regs *regs) if (unlikely(t->flags & PF_EXITING)) return; + if (!regs) { +#ifdef CONFIG_DEBUG_RSEQ + if (t->rseq && rseq_get_rseq_cs(t, &rseq_cs)) + goto error; +#endif + return; + } ret = rseq_ip_fixup(regs); if (unlikely(ret < 0)) goto error; > Thanks for looking into this ! > > Mathieu > > > return clear_rseq_cs(t); > > ret = rseq_need_restart(t, rseq_cs.flags); > > if (ret <= 0) > > -- > > 2.33.0.rc1.237.g0d66db33f3-goog > > -- > Mathieu Desnoyers > EfficiOS Inc. > http://www.efficios.com _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel