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 5ABD4C433E0 for ; Wed, 27 Jan 2021 15:20:57 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 26CCC207E8 for ; Wed, 27 Jan 2021 15:20:57 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1343576AbhA0PU2 (ORCPT ); Wed, 27 Jan 2021 10:20:28 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54226 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235531AbhA0POu (ORCPT ); Wed, 27 Jan 2021 10:14:50 -0500 Received: from mail-ed1-x52b.google.com (mail-ed1-x52b.google.com [IPv6:2a00:1450:4864:20::52b]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D385BC061574 for ; Wed, 27 Jan 2021 07:14:09 -0800 (PST) Received: by mail-ed1-x52b.google.com with SMTP id d22so2893163edy.1 for ; Wed, 27 Jan 2021 07:14:09 -0800 (PST) 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=65ksyZM56LYMi/17n5/nAjH2134nzomrwEiQi+8o7zE=; b=QvBUcDhfRre5rgXFZhYrYnO3VKwX8PbnCtBsVFGAG0PA13IkgQ2XRjN0X9y1v94zoB gBKtWqHyra8gcbf/JCH13pPvPwb9di+6XiLSEtS8n4a+Pyugks026dNW9xRTiMRVkGTv +4z8jRerwsiPf0fcYn2dv4T10Utb66B5Kc59R5bP5W7xttmBxRwM61JT66IJpb0RMYVZ s7TDJw9ZXJcUoyS+J0kUaB534ln2ybiroHCpOMIBnSaR6wPdV39WavvjOKfgCHHRTBAN xrLEy4rM6f6X6c8+fTA0nZbb/twPxjnmX1CkatKEcWHmVjkUoDndSbqDLp1sXYvuFhnx SBWQ== 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=65ksyZM56LYMi/17n5/nAjH2134nzomrwEiQi+8o7zE=; b=chdlYSxmh4iyQldI0jbhiNvHwU7TZmSZv/m3dzZwbAM1wHLkbuvCgpaUOxaF2NnA58 zeinmMAub17ZABiMDVZ9zevxmrcrCVSbmbinvbb4izJ6D/JomDGrCh8nuas/Ry94oc/b Tb6TyjjbINMr6aHLZ2At1ACOVDTLPwP7HYSuHbDykx+YG5K1ZnJoKtbRtLRmCo/soCp8 FNYdOABhfd1eWGilBEpfJbOuWDkbaKLFSLfURasxZPd0SraDPLpgIJHHEmEWnYhQRNpw I1VB069hwgWTsKMlg9MuyPNaizLHn07d3yt4+kWkPPweENpuO/xrrRfVBcHktug3tood 62hg== X-Gm-Message-State: AOAM533E0xkC7N+Qc9w1wTNjik9uc0EfnQiXOz3Q9SAeNrsBeG9kw0wO Kv70slIbAGTC/rT7xAIZ7d+7nA== X-Google-Smtp-Source: ABdhPJwVjoCGsw0QUtN6fAwKI2x8vq18sti3VRWHotgE12FpaSzKE03fMU244ooBacr70pUhd3FdIg== X-Received: by 2002:a05:6402:4310:: with SMTP id m16mr9573676edc.207.1611760448361; Wed, 27 Jan 2021 07:14:08 -0800 (PST) Received: from google.com ([2a00:79e0:2:11:1ea0:b8ff:fe79:fe73]) by smtp.gmail.com with ESMTPSA id m10sm1470468edi.54.2021.01.27.07.14.07 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 27 Jan 2021 07:14:07 -0800 (PST) Date: Wed, 27 Jan 2021 16:14:01 +0100 From: Piotr Figiel To: Mathieu Desnoyers Cc: Alexey Dobriyan , "Eric W. Biederman" , Andrew Morton , Kees Cook , Alexey Gladkov , Michel Lespinasse , Bernd Edlinger , Andrei Vagin , Alexander Viro , Peter Zijlstra , paulmck , Boqun Feng , linux-kernel , linux-fsdevel , Peter Oskolkov , Kamil Yurtsever , Chris Kennelly , Paul Turner Subject: Re: [PATCH v3] fs/proc: Expose RSEQ configuration Message-ID: References: <20210126185412.175204-1-figiel@google.com> <177374191.8780.1611694726862.JavaMail.zimbra@efficios.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <177374191.8780.1611694726862.JavaMail.zimbra@efficios.com> Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Jan 26, 2021 at 03:58:46PM -0500, Mathieu Desnoyers wrote: > ----- On Jan 26, 2021, at 1:54 PM, Piotr Figiel figiel@google.com wrote: > [...] > > diff --git a/kernel/rseq.c b/kernel/rseq.c > > index a4f86a9d6937..6aea67878065 100644 > > --- a/kernel/rseq.c > > +++ b/kernel/rseq.c > > @@ -322,8 +322,10 @@ SYSCALL_DEFINE4(rseq, struct rseq __user *, rseq, u32, > > rseq_len, > > ret = rseq_reset_rseq_cpu_id(current); > > if (ret) > > return ret; > > + task_lock(current); > > current->rseq = NULL; > > current->rseq_sig = 0; > > + task_unlock(current); > > return 0; > > } > > > > @@ -353,8 +355,10 @@ SYSCALL_DEFINE4(rseq, struct rseq __user *, rseq, u32, > > rseq_len, > > return -EINVAL; > > if (!access_ok(rseq, rseq_len)) > > return -EFAULT; > > + task_lock(current); > > current->rseq = rseq; > > current->rseq_sig = sig; > > + task_unlock(current); > > So AFAIU, the locks are there to make sure that whenever a user-space > thread reads that state through that new /proc file ABI, it observes > coherent "rseq" vs "rseq_sig" values. Yes, that was the intention. > However, I'm not convinced this is the right approach to consistency > here. > > Because if you add locking as done here, you ensure that the /proc > file reader sees coherent values, but between the point where those > values are read from kernel-space, copied to user-space, and then > acted upon by user-space, those can very well have become outdated if > the observed process runs concurrently. You are right here, but I think this comment is valid for most of the process information exported via procfs. The user can almost always make a time of check/time of use race when interacting with procfs. I agree that the locking added in v3 doesn't help much, but at least it does provide a well defined answer: i.e. at least in some point of time the effective configuration was as returned. It makes it a bit easier to document and reason about the file contents, compared to the inconsistent version. > So my understanding here is that the only non-racy way to effectively > use those values is to either read them from /proc/self/* (from the > thread owning the task struct), or to ensure that the thread is > stopped/frozen while the read is done. Constraining this solely to the owning thread I think is a bit too limiting. I think we could limit it to stopped threads but I don't think it eliminates the potential of time of check/time of use races for the user. In this shape as in v3 - it's up to the user to decide if there is a relevant risk of a race, if it's unwanted then the thread can be stopped with e.g. ptrace, cgroup freeze or SIGSTOP. Best regards, Piotr.