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 CDC2EC83F10 for ; Thu, 31 Aug 2023 15:27:12 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 636F5900003; Thu, 31 Aug 2023 11:27:12 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 5E62F8D0001; Thu, 31 Aug 2023 11:27:12 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 4AE71900003; Thu, 31 Aug 2023 11:27:12 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 379CF8D0001 for ; Thu, 31 Aug 2023 11:27:12 -0400 (EDT) Received: from smtpin24.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id CFC358031B for ; Thu, 31 Aug 2023 15:27:11 +0000 (UTC) X-FDA: 81184778262.24.5705E50 Received: from mail-yw1-f179.google.com (mail-yw1-f179.google.com [209.85.128.179]) by imf13.hostedemail.com (Postfix) with ESMTP id 03F6220008 for ; Thu, 31 Aug 2023 15:27:09 +0000 (UTC) Authentication-Results: imf13.hostedemail.com; dkim=pass header.d=paul-moore.com header.s=google header.b=UQVk7FmY; dmarc=pass (policy=none) header.from=paul-moore.com; spf=pass (imf13.hostedemail.com: domain of paul@paul-moore.com designates 209.85.128.179 as permitted sender) smtp.mailfrom=paul@paul-moore.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1693495630; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=3p4KVcplnkmAq97aJFuRfPluC00MQY2jsCtIut4Z1Gw=; b=Ky25h3XNrOM4ZNjb7jiQPV3yC38g9qXriQ1arTx1A+vTj6Wjl1czNobNbmWSCNVusql638 0GM3vkMnmLGrHism28sqNFuocyPwgN3BFrwkBy7OA0AL8uJsL/qT7bRBA+nHZcCfjIG4Ys z/dmJ9f+RAj1CHfaiywPh/W+4WKw/Gs= ARC-Authentication-Results: i=1; imf13.hostedemail.com; dkim=pass header.d=paul-moore.com header.s=google header.b=UQVk7FmY; dmarc=pass (policy=none) header.from=paul-moore.com; spf=pass (imf13.hostedemail.com: domain of paul@paul-moore.com designates 209.85.128.179 as permitted sender) smtp.mailfrom=paul@paul-moore.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1693495630; a=rsa-sha256; cv=none; b=Z12tuGAnHbi0HSlfu5r8d7FyePSfHCDb3FS0npaTcUthtVF6YoCSmqo6ijfxm5nUvHaSj2 OwN/vdvo6roUmB9qhBuoDEEsbhQDMkG6rUiLhNxOXSbzgauKe6QhzmAIfgqjapI8vFtlQ3 C7j8bxivWac9oniHnq705siYjxUEVkE= Received: by mail-yw1-f179.google.com with SMTP id 00721157ae682-58d9ba95c78so11094367b3.1 for ; Thu, 31 Aug 2023 08:27:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=paul-moore.com; s=google; t=1693495629; x=1694100429; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=3p4KVcplnkmAq97aJFuRfPluC00MQY2jsCtIut4Z1Gw=; b=UQVk7FmYHTfYsL9/s1JVS5+j4JHjIUO5sET+jrBkUiVZkenq8dXzLNT/Fs+tz+fpnJ RGYXOoBBvVIUu8d2rUG/19JXCiNIILBIEJqGdL73m0eOyRrbJzbFvzMs7vN07SUfm4PX xoiLU3A3MPJgxajU5GNH9WzGd/O8UAJ2ChtYnq4sVVIh97Im/XP3qHWNNBZMOCxW4maR 7hJkohu4wP+34z6uNxDyAFWJpHLfZqhgWWYKykZTV9A7fOICx3zwv/WcTLlI/QJ9hVA/ Ji6PWxdO94SmfKVX9pNmB1sNQfQSTySUoOHNWCkD64/n8Yfm7gspNSX2SNvceBr9+CsO 3bCw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1693495629; x=1694100429; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=3p4KVcplnkmAq97aJFuRfPluC00MQY2jsCtIut4Z1Gw=; b=dK8XT5kXaPIUCz7QAWPukzHLcIxC0j8eYK1NsuGOBoo1eAScb3aSkVKmRZnIRmchyf IjpKyeeRGcYHvEn6JUUH+a+1g8V2y9cMmHTzKNKywVwpMR1Me+celFYdwkXzsagKMVoS TZ8Se+8WBsLPQUS1ytdumPgGE0GkcjilN3UYbyUB+19jriknwtQeowpFVAMAmSMcJxWR xA991nsuRmZgtTipq/5Igf+Z+PrdsAjHpsKwO+fOHzsxrN3Auj74jWPxaB/EA5aUplB5 8d9geersT9b6Njale2A1l9VLPa9u1FhayCKFyPg2PzcHGGAQ5GG737E5AdQbW8cc3wuO dQ7g== X-Gm-Message-State: AOJu0YykTPtBof0Md+kmLfi5wmrYSyte2jY7veTv179RcTVjAAgbuzJM UByiT35iVZZpJuh/QJHB7uhzNOUdgzPeTdH4GrVT X-Google-Smtp-Source: AGHT+IEbAPJPHeshhRY/Lhxcw/6oRYRPRw32NnYy3eYgXlznsnIjt+TNPp9z+bpcymHvBZK/BtkQ6Ltb5kMj49p25ig= X-Received: by 2002:a81:6cd8:0:b0:58f:a19f:2b79 with SMTP id h207-20020a816cd8000000b0058fa19f2b79mr6137570ywc.9.1693495629007; Thu, 31 Aug 2023 08:27:09 -0700 (PDT) MIME-Version: 1.0 References: <20230831-nachverfolgen-meditation-dcde56b10df7@brauner> In-Reply-To: From: Paul Moore Date: Thu, 31 Aug 2023 11:26:58 -0400 Message-ID: Subject: Re: LSM hook ordering in shmem_mknod() and shmem_tmpfile()? To: Mimi Zohar Cc: Christian Brauner , Hugh Dickins , Al Viro , Andrew Morton , linux-security-module@vger.kernel.org, linux-fsdevel@vger.kernel.org, selinux@vger.kernel.org, linux-mm@kvack.org, linux-integrity@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: 03F6220008 X-Rspam-User: X-Rspamd-Server: rspam05 X-Stat-Signature: 5nr7575z8m9pq69wj1trqcocns7y3h3b X-HE-Tag: 1693495629-496590 X-HE-Meta: U2FsdGVkX19+sqY8wkKybLxr0ONHYL5eQt6+BcKZbhxq6YDBMybd323LvAfinZPEL/JlalfTdRIEllBffoUoRsVIYcpe/aAwIlYU4gt0geUfm4cyqlED+eHDYJNw7t6GtsgpJ2amgY2fyQkBTg7qdeCN7xXqXE2Yj7f+saqkLTaLCB5u3cFq+qb6MbBBb2k351L5gG1mPmvp/QdDtTPYdB92d4fBqnHb0GAIMDbOR4JEieXMqZ5v97JMWMUqlQd2cGUaO5iUMLqut7PvrEn/M5rdVNFB6RUfQkmdDVAdM5RCCRjLkGmB7wW0v4j6rfTwGcVW0yvOgOU308YY81aXiIkprYzF+rvqAtFfPVSKg/yxOZjnLYIXgxCA9m4kUR1+XFUGoN+LA6IQyV2XQTVQop+vNkBsCHxXYl/SgOVW7FKaH2bHyMH8XAheWRQsEydowBSNurpFQMqn9gGdVu89My86lyUywolMPjLyyIKb0N1MEfIYewIxp3v8ItDuI1JjgDN1fQL/vczViNxsDNMaTzQ5CWqQBUNf2SD3NfJk0QfBKbHZtS705Dfge1UAVHEBx0DDWyh8VlrDnc0btlCtF87T8UNne7qq/B4skMXN+r2kF+mJA43hbael8ut+IHIsovHWsvGuq7Q2NPM7YXBY3w+YB7ngcLeNECk1+1A2KxnoEyZipGd8U0pZt4LdKvukJiYl5JeEFHcPPo5LzGz2e36S0/WrWfpFOCnxH617QQzE7XcuG2rZMfRyOdRcz/g1Xi6DKkRV5o67GMD14rsJzQk78tzd1ht7isEkuKjLfs/o5ADyZ9AbzGXU2oKWOBko4KhEhLN2ONG22BeusTy+XFNGJABQ9WDHLvelZZZbTGEKukYC3O0aHh1zVF1dGD7Z9eh3dE9tEMzTsgdhHPz2lSDUBPtl+6P2JTIh7ZTeJ8g4X4BRhuj98b6Sl3hxJ2DTy9mArGtO4Y1ud+/xnyQ jdvY8w7g XHBWT1rJWGFyEeE7OFXqKnvq4/VzaTP0izROimI1e76j81kV1Xz/kZqcFBLueJPz13HXUSPB+fU+AMaFfnsCTmj+wt1ACTCF9+1AcJh8m/R3+8wjHZ4hvYFtMQuFHo5my1jMIK69W6Jv8potHjZxbFORUwE1BTNl09bVsV6cNlsZQ8AYk4hnWhH0o8e5vPVOUsKwB8SsOuTmuvAq7ieyc1XOmjXQGKBvyc1sTTy7G1uZPT2drU0OikiAATDf5gIl4B/jeoGROTUHIhj+/96TdY55dLoKmf/LUAss2FgqY8/NQ6Eg/XK/5I6trt4y8GvU3QvRpN1KBVzG0lZaVtOl797qH7+BrAKny3pezndDzW9kCtfIhvW7F2C8ylYYPbe/FDvYLcHs617jMAQpH2xz4e97M3+TE5FrMFV5S 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, Aug 31, 2023 at 11:13=E2=80=AFAM Mimi Zohar w= rote: > On Thu, 2023-08-31 at 14:36 +0200, Christian Brauner wrote: > > On Thu, Aug 31, 2023 at 02:19:20AM -0700, Hugh Dickins wrote: > > > On Wed, 30 Aug 2023, Paul Moore wrote: > > > > > > > Hello all, > > > > > > > > While looking at some recent changes in mm/shmem.c I noticed that t= he > > > > ordering between simple_acl_create() and > > > > security_inode_init_security() is different between shmem_mknod() a= nd > > > > shmem_tmpfile(). In shmem_mknod() the ACL call comes before the LS= M > > > > hook, and in shmem_tmpfile() the LSM call comes before the ACL call= . > > > > > > > > Perhaps this is correct, but it seemed a little odd to me so I want= ed > > > > to check with all of you to make sure there is a good reason for th= e > > > > difference between the two functions. Looking back to when > > > > shmem_tmpfile() was created ~2013 I don't see any explicit mention = as > > > > to why the ordering is different so I'm looking for a bit of a sani= ty > > > > check to see if I'm missing something obvious. > > > > > > > > My initial thinking this morning is that the > > > > security_inode_init_security() call should come before > > > > simple_acl_create() in both cases, but I'm open to different opinio= ns > > > > on this. > > > > > > Good eye. The crucial commit here appears to be Mimi's 3.11 commit > > > 37ec43cdc4c7 "evm: calculate HMAC after initializing posix acl on tmp= fs" > > > which intentionally moved shmem_mknod()'s generic_acl_init() up befor= e > > > the security_inode_init_security(), around the same time as Al was > > > copying shmem_mknod() to introduce shmem_tmpfile(). > > > > > > I'd have agreed with you, Paul, until reading Mimi's commit: > > > now it looks more like shmem_tmpfile() is the one to be changed, > > > except (I'm out of my depth) maybe it's irrelevant on tmpfiles. > > > > POSIX ACLs generally need to be set first as they are may change inode > > properties that security_inode_init_security() may rely on to be stable= . > > That specifically incudes inode->i_mode: > > > > * If the filesystem doesn't support POSIX ACLs then the umask is > > stripped in the VFS before it ever gets to the filesystems. For such > > cases the order of *_init_security() and setting POSIX ACLs doesn't > > matter. > > * If the filesystem does support POSIX ACLs and the directory of the > > resulting file does have default POSIX ACLs with mode settings then > > the inode->i_mode will be updated. > > * If the filesystem does support POSIX ACLs but the directory doesn't > > have default POSIX ACLs the umask will be stripped. > > > > (roughly from memory) > > > > If tmpfs is compiled with POSIX ACL support the mode might change and i= f > > anything in *_init_security() relies on inode->i_mode being stable it > > needs to be called after they have been set. > > > > EVM hashes do use the mode and the hash gets updated when POSIX ACLs ar= e > > changed - which caused me immense pain when I redid these codepaths las= t > > year. > > > > IMHO, the easiest fix really is to lump all this together for all > > creation paths. This is what most filesystems do. For examples, see > > > > xfs_generic_create() > > -> posix_acl_create(&mode) > > -> xfs_create{_tmpfile}(mode) > > -> xfs_inode_init_security() > > > > or > > > > __ext4_new_inode() > > -> ext4_init_acl() > > -> ext4_init_security() > > Agreed. Thanks, Hugh, Christian for the clear explanation. Yes, thanks all. I figured something was a little wonky but wasn't smart enough to know the correct fix. So .... who wants to submit a patch? --=20 paul-moore.com