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 7E56CC77B75 for ; Tue, 23 May 2023 16:36:11 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id B229D900003; Tue, 23 May 2023 12:36:10 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id AABAA900002; Tue, 23 May 2023 12:36:10 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 97F05900003; Tue, 23 May 2023 12:36:10 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 83A53900002 for ; Tue, 23 May 2023 12:36:10 -0400 (EDT) Received: from smtpin09.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 480091A072C for ; Tue, 23 May 2023 16:36:10 +0000 (UTC) X-FDA: 80822072100.09.53B169D Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf09.hostedemail.com (Postfix) with ESMTP id 960B4140009 for ; Tue, 23 May 2023 16:36:08 +0000 (UTC) Authentication-Results: imf09.hostedemail.com; dkim=none; dmarc=fail reason="SPF not aligned (relaxed), No valid DKIM" header.from=arm.com (policy=none); spf=pass (imf09.hostedemail.com: domain of cmarinas@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=cmarinas@kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1684859768; a=rsa-sha256; cv=none; b=p35v/wWc8woGX8G55zzbYKkwBnu5GG6Kx1InZ1bhjo/NJWfjvTtWe+axx6VsY1+M1VBeZn wq17SmWy01RFvVdzP/nEMt+uYiEe3VTcuu4ILJvxqHDCv0yOlXOZSR/J9VwoFxcFpT5lto GDsFc/e83BasAb/30U8Wr3jVv0QmabI= ARC-Authentication-Results: i=1; imf09.hostedemail.com; dkim=none; dmarc=fail reason="SPF not aligned (relaxed), No valid DKIM" header.from=arm.com (policy=none); spf=pass (imf09.hostedemail.com: domain of cmarinas@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=cmarinas@kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1684859768; 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; bh=yrz3BYEJLxoFwfRvsTAIfNNQNxL4cA08bf7BN09IRPg=; b=qdPC6ET1oH9213untaPehC5lMkGFIkhnIuI99onzKUxeB2/7sLNqlarHlBF83v9KMqs9D3 bSNLu9bD8CTyEuEt+AC0UHIj7J2yLLio/ZmEa2sm5fE9UUnAzZcTAafvhdhLXgpVSbT+UF 1TlsiKORfmXBLGxr0Gfni2CWscwjPas= Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 4802262CD5; Tue, 23 May 2023 16:36:07 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id CDD67C433EF; Tue, 23 May 2023 16:36:03 +0000 (UTC) Date: Tue, 23 May 2023 17:36:00 +0100 From: Catalin Marinas To: Florent Revest Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org, akpm@linux-foundation.org, anshuman.khandual@arm.com, joey.gouly@arm.com, mhocko@suse.com, keescook@chromium.org, david@redhat.com, peterx@redhat.com, izbyshev@ispras.ru, broonie@kernel.org, szabolcs.nagy@arm.com, kpsingh@kernel.org, gthelen@google.com, toiwoton@gmail.com Subject: Re: [PATCH v2 4/5] mm: Add a NO_INHERIT flag to the PR_SET_MDWE prctl Message-ID: References: <20230517150321.2890206-1-revest@chromium.org> <20230517150321.2890206-5-revest@chromium.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230517150321.2890206-5-revest@chromium.org> X-Rspamd-Server: rspam08 X-Rspamd-Queue-Id: 960B4140009 X-Stat-Signature: mdhuttd3xt5aks4mf7c8iorss67736qr X-Rspam-User: X-HE-Tag: 1684859768-784260 X-HE-Meta: U2FsdGVkX1/M9Xl5IzWBw392qOTSv2Mv8An6HU3nVWTb/MO9CWLWVQW/4/Jhg95La5Zd28xmpNQp8/FetGfOrt5z9ZGwh0BWJzoPKva8RdQLA3IaTs39ksDVdrUFZMy+77G1PeKKkcPRq/vlTTvhVkNKpevPCpr1HkF0qPNNspe4ME6NAwQ3q0hlAvs7EHQcgVliiWDm/xqMoXT8o7niCD6GUhj8+4QnabeCW3400wVtpDwCJVbhaDs/geSLU+qa8kTf7VX9Vjx/YlDN7Vbgb8MjmoK4sYPhOcOxqoynI7t1rMb/R/SARKMeDxhgNWjdQOSTL2Z2CrvHxfWt7PO96T8iTtamubwxEYOB9tOYyUNuKaWEvhhofeSmWO/hnZuGKSwc/GvCYXXzn0O58ihiOVI/K5uOk/fStkJjJ0Xf7hEdlPWKH3upUjl1cuOuz0/FUBI+4gaFZO93oPNbc1agjDOuI+Nbryy4HUzgiaw24ei3jusg9kncMSJsTwL0WUBM1XB+pskW+Fln60RDx3V+NFImw7aW7xGy6HC9bmjAGknf+z1JR0SHh9CdqbOF/eegYYdTXuc0Lf9Vu8qKqBPMZZkqcCkzl7PSLVgARFjA1qZZufzxwlw2rmUzCQl3i4dV632y+cee9liRaQObGyjSlJF2Bb557VpZ8ih1JsZ//mHHQK2S9wKx/aKcsyG2Hv8uaMYNybyM7t3qDxoEoIzAh20eNGnBuIu97XPai8vY7IAj80S3Nvfueqx3SiDfy4oE3Kx62TED/0UMXhfISUlchU4c/fGlXioiuxnSsrOxLpANWFVkHSM4TLyGVN7LPYNM/0ZHAevBDS+fWuExQoduaGEi8TimqnA6wwjQqkKyaTh8jY2fedXXsMA7byeRoPxOY5bdSUFkpu9xugIOThrPKg4xIwd2tUx9iXo8C4U7o4TrKArVwDID/3g3gB0NAGM1UkLSnxQjOXfSi+5J36O zKFv9vHJ g5RDjTVSQK3MP0sUc8ZbARSMtsPOXZQNgqbh8Ur9+EajAPqQSnr5RvyvtWNbCN22wHrRE6yr/25p5200f/iEOlxOjQJPt0XdQhF2gxLYvsQWM8GsJscs5nCxjQM3vHQxSEfF8rI9kE+lN79X2qtj90m2rViUtmOsY/IMM+A7l2NRbGRHCGKcevBWZgwyLEpKkl8PmA4gIgUsOBWhTVlxPtiDQ51pstTkKZO3xkDghKc0K6wfZcK5YTaznTGmjc8PdJLEjsr3rgo7m9jAqfmV6uvGwt9IcxnmAE3L+U8tA/zGTy9ckuLRXS3Pz7jG8HdAPSpYQ/FS9WPHqMUpW+bS8/xpU1JkmQtUR3zI1bmoLw+z4MLXyVVvD/b2S9u/KyaxUwbKH86flDrhHBhzBnkB8+mFK8BSGXpXZtMHO7mkwOWKxwyie18pKZmUYfEArp/RCjsAztRQCQF9NYXTvqqLBRhC9Fg== 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 Wed, May 17, 2023 at 05:03:20PM +0200, Florent Revest wrote: > diff --git a/include/linux/sched/coredump.h b/include/linux/sched/coredump.h > index 0ee96ea7a0e9..11f5e3dacb4e 100644 > --- a/include/linux/sched/coredump.h > +++ b/include/linux/sched/coredump.h > @@ -91,4 +91,14 @@ static inline int get_dumpable(struct mm_struct *mm) > MMF_DISABLE_THP_MASK | MMF_HAS_MDWE_MASK) > > #define MMF_VM_MERGE_ANY 29 > +#define MMF_HAS_MDWE_NO_INHERIT 30 > + > +#define MMF_INIT_FLAGS(flags) ({ \ > + unsigned long new_flags = flags; \ > + if (new_flags & (1UL << MMF_HAS_MDWE_NO_INHERIT)) \ > + new_flags &= ~((1UL << MMF_HAS_MDWE) | \ > + (1UL << MMF_HAS_MDWE_NO_INHERIT)); \ > + new_flags & MMF_INIT_MASK; \ > +}) A function is better indeed, not sure who came up with this macro idea ;). > diff --git a/kernel/sys.c b/kernel/sys.c > index 339fee3eff6a..320eae3b12ab 100644 > --- a/kernel/sys.c > +++ b/kernel/sys.c > @@ -2368,9 +2368,25 @@ static inline int prctl_set_mdwe(unsigned long bits, unsigned long arg3, > if (arg3 || arg4 || arg5) > return -EINVAL; > > - if (bits & ~(PR_MDWE_REFUSE_EXEC_GAIN)) > + if (bits & ~(PR_MDWE_REFUSE_EXEC_GAIN | PR_MDWE_NO_INHERIT)) > return -EINVAL; > > + /* NO_INHERIT only makes sense with REFUSE_EXEC_GAIN */ > + if (bits & PR_MDWE_NO_INHERIT && !(bits & PR_MDWE_REFUSE_EXEC_GAIN)) > + return -EINVAL; > + > + /* Can't gain NO_INHERIT from !NO_INHERIT */ > + if (bits & PR_MDWE_NO_INHERIT && > + test_bit(MMF_HAS_MDWE, ¤t->mm->flags) && > + !test_bit(MMF_HAS_MDWE_NO_INHERIT, ¤t->mm->flags)) > + return -EPERM; > + > + if (bits & PR_MDWE_NO_INHERIT) > + set_bit(MMF_HAS_MDWE_NO_INHERIT, ¤t->mm->flags); > + else if (test_bit(MMF_HAS_MDWE_NO_INHERIT, ¤t->mm->flags) > + && !(bits & PR_MDWE_REFUSE_EXEC_GAIN)) > + return -EPERM; /* Cannot unset the flag */ Is this about not unsetting the MMF_HAS_MDWE bit? We already have a check further down that covers this case. Related to this, do we want to allow unsetting MMF_HAS_MDWE_NO_INHERIT? It looks like it can't be unset but no error either. The above check, IIUC, looks more like ensuring we don't clear MMF_HAS_MDWE. Maybe we should tighten the logic here a bit and not allow any changes after the initial flag setting: current->mm->flags == 0, we allow: bits == 0 or bits == PR_MDWE_REFUSE_EXEC_GAIN or bits == PR_MDWE_REFUSE_EXEC_GAIN | PR_MDWE_NO_INHERIT current->mm->flags != 0 (some bits were set), we only allow the exactly the same bit combination or -EPERM. So basically build the flags based on the PR_* input bits and compare them with current->mm->flags when not 0, return -EPERM if different. I think this preserves the ABI as we only have a single bit currently and hopefully makes the logic here easier to parse. > + > if (bits & PR_MDWE_REFUSE_EXEC_GAIN) > set_bit(MMF_HAS_MDWE, ¤t->mm->flags); > else if (test_bit(MMF_HAS_MDWE, ¤t->mm->flags)) > @@ -2385,8 +2401,10 @@ static inline int prctl_get_mdwe(unsigned long arg2, unsigned long arg3, > if (arg2 || arg3 || arg4 || arg5) > return -EINVAL; > > - return test_bit(MMF_HAS_MDWE, ¤t->mm->flags) ? > - PR_MDWE_REFUSE_EXEC_GAIN : 0; > + return (test_bit(MMF_HAS_MDWE, ¤t->mm->flags) ? > + PR_MDWE_REFUSE_EXEC_GAIN : 0) | > + (test_bit(MMF_HAS_MDWE_NO_INHERIT, ¤t->mm->flags) ? > + PR_MDWE_NO_INHERIT : 0); > } Just personal preference, use explicit 'if' blocks and add bits to a local variable variable than multiple ternary operators. -- Catalin