From patchwork Sat Mar 18 10:15:15 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Amir Goldstein X-Patchwork-Id: 13179728 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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id C8595C74A5B for ; Sat, 18 Mar 2023 10:15:45 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229868AbjCRKPm (ORCPT ); Sat, 18 Mar 2023 06:15:42 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54762 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229588AbjCRKPl (ORCPT ); Sat, 18 Mar 2023 06:15:41 -0400 Received: from mail-wm1-x32e.google.com (mail-wm1-x32e.google.com [IPv6:2a00:1450:4864:20::32e]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id DDBAD2410D; Sat, 18 Mar 2023 03:15:38 -0700 (PDT) Received: by mail-wm1-x32e.google.com with SMTP id bh21-20020a05600c3d1500b003ed1ff06fb0so4717598wmb.3; Sat, 18 Mar 2023 03:15:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1679134537; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=PP8hrBv8Zg8teY5qFhszUF4+5kq7WXrNzkvw8H5L9wU=; b=kPRmcTrxrivd2xuDLHt5cE6iYYvBIHf4nvCdgVa+M+57aq9hnoNz7SrURAVTqmcwPg iXCx7XbGglaOK3BtBjKOVMsgnSzpKRusVDL/ERfbg56PZ7tRy7qtTIKcmIw4zqVl8fmm 84XVgCIMZk81KcVFqz9lcpiNqmzaRjAJpbdxq+ks06+yy0f0oRJLtngFozSegNtu5SHO f2pWlOZAVvH3Gl1NCF18uFJS50b8QLPty/J82SovpwM6Xan2kXTkJJEBEPfCUjp9zgZL Bd9r203XrnqdAw42//OsNcerPTq67OEjWwklA0e4O6MHXr/pmI4uM7azyfDVz1WaMFzy uSMg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1679134537; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=PP8hrBv8Zg8teY5qFhszUF4+5kq7WXrNzkvw8H5L9wU=; b=ct83WwT1lZJ2Y4mzP66acvwZ/UohlCbfJ1I52yzHcSqcXXux9iFF5JGLkqCflzCb+g 5nwfjSzyzglNX9+vkl8JjbdGjAVcGvYbMkwQw3L452spXizM1qZ0Ji2otmpkXMB0XDrd gVYT9WYUU3r/rx8TIkF3y55iVZwtGoarXla+T4rO+MGf5j9V8B3MNmDOESyHUxycFxoY HEPf1R4yEyOr5Ec3myGuwZlfz/vtwEVfnwsUsZoXmkeXoaj3zF6kxgnzlHFI/4FIOsds ahKdcsiIBBxoLMv35Uqd/1B0ZxAhnyKJUH36l5uSTY/GGdTm7ZiGIHtTpC22cXYw1taC hdNQ== X-Gm-Message-State: AO0yUKXQMMoB1zUOF1CBrGdsa6kFCoorKUzKRfORBeqw3pK700rvsG0B tatVdU5DhIFDY82tBfcMQaA= X-Google-Smtp-Source: AK7set/eTR5iXiGk1JpNxuDJvoCXQSBxXjSju76oRZWaqFETPcKCm/WhdvKoMixuwGaiLannhivCUg== X-Received: by 2002:a05:600c:3c9e:b0:3e1:f8af:8772 with SMTP id bg30-20020a05600c3c9e00b003e1f8af8772mr26614966wmb.9.1679134537368; Sat, 18 Mar 2023 03:15:37 -0700 (PDT) Received: from amir-ThinkPad-T480.lan ([5.29.249.86]) by smtp.gmail.com with ESMTPSA id v26-20020a05600c215a00b003eafc47eb09sm4333965wml.43.2023.03.18.03.15.35 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 18 Mar 2023 03:15:36 -0700 (PDT) From: Amir Goldstein To: Greg Kroah-Hartman Cc: Sasha Levin , "Darrick J . Wong" , Leah Rumancik , Chandan Babu R , Christian Brauner , linux-fsdevel@vger.kernel.org, linux-xfs@vger.kernel.org, stable@vger.kernel.org, Dave Chinner , Christoph Hellwig , Dave Chinner Subject: [PATCH 5.10 01/15] xfs: don't assert fail on perag references on teardown Date: Sat, 18 Mar 2023 12:15:15 +0200 Message-Id: <20230318101529.1361673-2-amir73il@gmail.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20230318101529.1361673-1-amir73il@gmail.com> References: <20230318101529.1361673-1-amir73il@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org From: Dave Chinner commit 5b55cbc2d72632e874e50d2e36bce608e55aaaea upstream. [backport for 5.10.y, prior to perag refactoring in v5.14] Not fatal, the assert is there to catch developer attention. I'm seeing this occasionally during recoveryloop testing after a shutdown, and I don't want this to stop an overnight recoveryloop run as it is currently doing. Convert the ASSERT to a XFS_IS_CORRUPT() check so it will dump a corruption report into the log and cause a test failure that way, but it won't stop the machine dead. Signed-off-by: Dave Chinner Reviewed-by: Darrick J. Wong Reviewed-by: Christoph Hellwig Signed-off-by: Dave Chinner Signed-off-by: Amir Goldstein --- fs/xfs/xfs_mount.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c index a2a5a0fd9233..402cf828cc91 100644 --- a/fs/xfs/xfs_mount.c +++ b/fs/xfs/xfs_mount.c @@ -126,7 +126,6 @@ __xfs_free_perag( { struct xfs_perag *pag = container_of(head, struct xfs_perag, rcu_head); - ASSERT(atomic_read(&pag->pag_ref) == 0); kmem_free(pag); } @@ -145,7 +144,7 @@ xfs_free_perag( pag = radix_tree_delete(&mp->m_perag_tree, agno); spin_unlock(&mp->m_perag_lock); ASSERT(pag); - ASSERT(atomic_read(&pag->pag_ref) == 0); + XFS_IS_CORRUPT(pag->pag_mount, atomic_read(&pag->pag_ref) != 0); xfs_iunlink_destroy(pag); xfs_buf_hash_destroy(pag); call_rcu(&pag->rcu_head, __xfs_free_perag); From patchwork Sat Mar 18 10:15:16 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Amir Goldstein X-Patchwork-Id: 13179729 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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2CF50C7619A for ; Sat, 18 Mar 2023 10:15:46 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229588AbjCRKPn (ORCPT ); Sat, 18 Mar 2023 06:15:43 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54772 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229878AbjCRKPm (ORCPT ); Sat, 18 Mar 2023 06:15:42 -0400 Received: from mail-wm1-x32c.google.com (mail-wm1-x32c.google.com [IPv6:2a00:1450:4864:20::32c]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C9876234CE; Sat, 18 Mar 2023 03:15:40 -0700 (PDT) Received: by mail-wm1-x32c.google.com with SMTP id az3-20020a05600c600300b003ed2920d585so6424255wmb.2; Sat, 18 Mar 2023 03:15:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1679134539; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=h7e309zG3GVSY+X1IZJ2K6Xi6wPSGKzC+UwjI2avX+I=; b=L0vDvTpIwv5eQKRz5a/a6ZE4ctlqIpzt6rqiXD4Zq5xMWmWA+FXMum7wJlrkQu5KXB 7VZ9/Wf0tC0mvm+qwB1DfpYZAyH2757yVHcBRG54v9ZemST/Z2vHOKxSoLKYK8w93L6R HfdCbV2alDp1NmJ2LDFGOdAo6Muxea9lEzeeRodBneg9J2aNv9onECdTK/aS7uVAHOg3 79ue1WGS8mYrcmV4AKnYZFmD+uue9VEHeH8ZqV6dsbvJAtDS/kdA1TuxqpAeN498kD3B Q03YaBN6R7YgTNOoA7+dZip97zno70jH4f3ru9X0gIAAiqPZf7uKcHS5Gd8C8ixfQV1c IcVg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1679134539; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=h7e309zG3GVSY+X1IZJ2K6Xi6wPSGKzC+UwjI2avX+I=; b=4S61lDFVu7rBxPL+AePNMLHj7i7NUa0PLU32bOs7y/iKmqkTJhaLmjT9tBITV7x0TL 5qaAA44PvRj9jRM5y1zvkLTpUxyWmVUy+J4foiqyZTnaK/CrlNvJxkgne99A+0StQsYd Luj+Kl7rleatlNBxna8lTm3n6hs00TkLYUzjgx3AoPXiWi67YHQDcYbpEE8vQ36/JqU7 fUzOUoGLynhpoAQPGAagxJINWgvqrbspfAlBAoSxaUEO2pgepn4ihbv0+Qi8AL77A9bB Xx0SR/824oXLbK+bcmnuX2iUDYSiLFzyGtXr3JaoE98pkrl3K3is7g+/IbxD6v7oHQuu KPcg== X-Gm-Message-State: AO0yUKXNe/PBW6e5d9tl0L5vWJ9cX+dLuABKMG5HISLzfhSmfOjt4+ZV z2N7ohQI/n0R9Q+aNhSmvoOs9+8peYc= X-Google-Smtp-Source: AK7set/c4cQj55J9eWa86RvyQQHY8Tyojdv7maBL6B5LBjVTzyjB+wy7aXEQLVVH/KQu5jY9SyaXqw== X-Received: by 2002:a05:600c:4592:b0:3e2:201a:5bcc with SMTP id r18-20020a05600c459200b003e2201a5bccmr26656277wmo.33.1679134539005; Sat, 18 Mar 2023 03:15:39 -0700 (PDT) Received: from amir-ThinkPad-T480.lan ([5.29.249.86]) by smtp.gmail.com with ESMTPSA id v26-20020a05600c215a00b003eafc47eb09sm4333965wml.43.2023.03.18.03.15.37 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 18 Mar 2023 03:15:38 -0700 (PDT) From: Amir Goldstein To: Greg Kroah-Hartman Cc: Sasha Levin , "Darrick J . Wong" , Leah Rumancik , Chandan Babu R , Christian Brauner , linux-fsdevel@vger.kernel.org, linux-xfs@vger.kernel.org, stable@vger.kernel.org, Christoph Hellwig , Dave Chinner , Dave Chinner Subject: [PATCH 5.10 02/15] xfs: purge dquots after inode walk fails during quotacheck Date: Sat, 18 Mar 2023 12:15:16 +0200 Message-Id: <20230318101529.1361673-3-amir73il@gmail.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20230318101529.1361673-1-amir73il@gmail.com> References: <20230318101529.1361673-1-amir73il@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org From: "Darrick J. Wong" commit 86d40f1e49e9a909d25c35ba01bea80dbcd758cb upstream. [add XFS_QMOPT_QUOTALL flag to xfs_qm_dqpurge_all() for 5.10.y backport] xfs/434 and xfs/436 have been reporting occasional memory leaks of xfs_dquot objects. These tests themselves were the messenger, not the culprit, since they unload the xfs module, which trips the slub debugging code while tearing down all the xfs slab caches: ============================================================================= BUG xfs_dquot (Tainted: G W ): Objects remaining in xfs_dquot on __kmem_cache_shutdown() ----------------------------------------------------------------------------- Slab 0xffffea000606de00 objects=30 used=5 fp=0xffff888181b78a78 flags=0x17ff80000010200(slab|head|node=0|zone=2|lastcpupid=0xfff) CPU: 0 PID: 3953166 Comm: modprobe Tainted: G W 5.18.0-rc6-djwx #rc6 d5824be9e46a2393677bda868f9b154d917ca6a7 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS ?-20171121_152543-x86-ol7-builder-01.us.oracle.com-4.el7.1 04/01/2014 Since we don't generally rmmod the xfs module between fstests, this means that xfs/434 is really just the canary in the coal mine -- something leaked a dquot, but we don't know who. After days of pounding on fstests with kmemleak enabled, I finally got it to spit this out: unreferenced object 0xffff8880465654c0 (size 536): comm "u10:4", pid 88, jiffies 4294935810 (age 29.512s) hex dump (first 32 bytes): 60 4a 56 46 80 88 ff ff 58 ea e4 5c 80 88 ff ff `JVF....X..\.... 00 e0 52 49 80 88 ff ff 01 00 01 00 00 00 00 00 ..RI............ backtrace: [] xfs_dquot_alloc+0x2c/0x530 [xfs] [] xfs_qm_dqread+0x6f/0x330 [xfs] [] xfs_qm_dqget+0x132/0x4e0 [xfs] [] xfs_qm_quotacheck_dqadjust+0xa0/0x3e0 [xfs] [] xfs_qm_dqusage_adjust+0x35d/0x4f0 [xfs] [] xfs_iwalk_ag_recs+0x348/0x5d0 [xfs] [] xfs_iwalk_run_callbacks+0x273/0x540 [xfs] [] xfs_iwalk_ag+0x5ed/0x890 [xfs] [] xfs_iwalk_ag_work+0xff/0x170 [xfs] [] xfs_pwork_work+0x79/0x130 [xfs] [] process_one_work+0x672/0x1040 [] worker_thread+0x59b/0xec0 [] kthread+0x29e/0x340 [] ret_from_fork+0x1f/0x30 Now we know that quotacheck is at fault, but even this report was canaryish -- it was triggered by xfs/494, which doesn't actually mount any filesystems. (kmemleak can be a little slow to notice leaks, even with fstests repeatedly whacking it to look for them.) Looking at the *previous* fstest, however, showed that the test run before xfs/494 was xfs/117. The tipoff to the problem is in this excerpt from dmesg: XFS (sda4): Quotacheck needed: Please wait. XFS (sda4): Metadata corruption detected at xfs_dinode_verify.part.0+0xdb/0x7b0 [xfs], inode 0x119 dinode XFS (sda4): Unmount and run xfs_repair XFS (sda4): First 128 bytes of corrupted metadata buffer: 00000000: 49 4e 81 a4 03 02 00 00 00 00 00 00 00 00 00 00 IN.............. 00000010: 00 00 00 01 00 00 00 00 00 90 57 54 54 1a 4c 68 ..........WTT.Lh 00000020: 81 f9 7d e1 6d ee 16 00 34 bd 7d e1 6d ee 16 00 ..}.m...4.}.m... 00000030: 34 bd 7d e1 6d ee 16 00 00 00 00 00 00 00 00 00 4.}.m........... 00000040: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ 00000050: 00 00 00 02 00 00 00 00 00 00 00 00 96 80 f3 ab ................ 00000060: ff ff ff ff da 57 7b 11 00 00 00 00 00 00 00 03 .....W{......... 00000070: 00 00 00 01 00 00 00 10 00 00 00 00 00 00 00 08 ................ XFS (sda4): Quotacheck: Unsuccessful (Error -117): Disabling quotas. The dinode verifier decided that the inode was corrupt, which causes iget to return with EFSCORRUPTED. Since this happened during quotacheck, it is obvious that the kernel aborted the inode walk on account of the corruption error and disabled quotas. Unfortunately, we neglect to purge the dquot cache before doing that, which is how the dquots leaked. The problems started 10 years ago in commit b84a3a, when the dquot lists were converted to a radix tree, but the error handling behavior was not correctly preserved -- in that commit, if the bulkstat failed and usrquota was enabled, the bulkstat failure code would be overwritten by the result of flushing all the dquots to disk. As long as that succeeds, we'd continue the quota mount as if everything were ok, but instead we're now operating with a corrupt inode and incorrect quota usage counts. I didn't notice this bug in 2019 when I wrote commit ebd126a, which changed quotacheck to skip the dqflush when the scan doesn't complete due to inode walk failures. Introduced-by: b84a3a96751f ("xfs: remove the per-filesystem list of dquots") Fixes: ebd126a651f8 ("xfs: convert quotacheck to use the new iwalk functions") Signed-off-by: Darrick J. Wong Reviewed-by: Christoph Hellwig Reviewed-by: Dave Chinner Signed-off-by: Dave Chinner Signed-off-by: Amir Goldstein --- fs/xfs/xfs_qm.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c index 64e5da33733b..3c17e0c0f816 100644 --- a/fs/xfs/xfs_qm.c +++ b/fs/xfs/xfs_qm.c @@ -1318,8 +1318,15 @@ xfs_qm_quotacheck( error = xfs_iwalk_threaded(mp, 0, 0, xfs_qm_dqusage_adjust, 0, true, NULL); - if (error) + if (error) { + /* + * The inode walk may have partially populated the dquot + * caches. We must purge them before disabling quota and + * tearing down the quotainfo, or else the dquots will leak. + */ + xfs_qm_dqpurge_all(mp, XFS_QMOPT_QUOTALL); goto error_return; + } /* * We've made all the changes that we need to make incore. Flush them From patchwork Sat Mar 18 10:15:17 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Amir Goldstein X-Patchwork-Id: 13179730 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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 705C4C61DA4 for ; Sat, 18 Mar 2023 10:15:48 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229892AbjCRKPq (ORCPT ); Sat, 18 Mar 2023 06:15:46 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54798 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229886AbjCRKPo (ORCPT ); Sat, 18 Mar 2023 06:15:44 -0400 Received: from mail-wm1-x332.google.com (mail-wm1-x332.google.com [IPv6:2a00:1450:4864:20::332]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 92FFB2410D; Sat, 18 Mar 2023 03:15:42 -0700 (PDT) Received: by mail-wm1-x332.google.com with SMTP id fm20-20020a05600c0c1400b003ead37e6588so6404969wmb.5; Sat, 18 Mar 2023 03:15:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1679134541; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=F/UNgWoj0g/ujTFM4Ms+u1ZVdhi+Be7I9bPF4qnLMhs=; b=osyEM8hrCwaVW8M7d99riQCpQc4kSf5sIGHDpZBINEcfro91poej+vdXmHBRoDKEmS 8dXchbztQXIf7otydjIHzCcB5TD/1hTvhs+pDuyr9JUBPJoM1+y70ztCoi6KGu0cbZta eo4RWEqnjAhGoj293BKi1R3fMPTFR55Wqa8ZG5UhY2Mdj/nXkI6PIVeOLi+g+pyUl09Y gB+YyjzEXQWIe+owIesZ9C8BJvoig/uHOWCitffcuhjvXcGm9UN43i9Tv/Zr/v6jA8ZN +a2j5EjIUsqcKKcMGF1bAvE/3xHX8gT/zagj7lwbzJVXVrr95f66Tw00RjCCto+lY0YC +SKw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1679134541; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=F/UNgWoj0g/ujTFM4Ms+u1ZVdhi+Be7I9bPF4qnLMhs=; b=zNt4RBJUq+0rMyUougvCQfveUt4ZvSxaEMfOqaErwFkhZGlPr7+lNQXxtgZuXNde6W bx427WcHE62IstoBaqnugeYx5FEmJd8fHGrOHVMkGZSfu4O0zvxgXndai/8Wkxh5sRGU XlHBZ4qRhAVewucjGmIRqunD6MonyJpCiNc/tkgFkPLNBwscQusm4Y5wReTD38eqo0Pe rVsipWDS4ukYu8+hub7beDZ9h+gTNXVXRKZp2QbLhj1l/CMPGKs51mxrccRHMCxrtYR1 qY+799kjLKw4EEj1LNzryndoHCSzJydSkXv2NJInU48Ev05tRalx5xd6o9pyke/HM4IT /YtQ== X-Gm-Message-State: AO0yUKUVrHn4MMrdbcumwFXSqGmsy2pOUDsyK1UgVKYtH8qhe1ZwUnxH 6MsEYTHsjv2JH5HSUANwOTY= X-Google-Smtp-Source: AK7set/RZLQJ59PCeA1JThPw2X8Kb4Looy4UYyMqmcaJ1Av5jypBMoYBdqPNpBxgT0xzp8MehnyuJg== X-Received: by 2002:a05:600c:4691:b0:3ed:33a1:ba8e with SMTP id p17-20020a05600c469100b003ed33a1ba8emr11575252wmo.1.1679134540778; Sat, 18 Mar 2023 03:15:40 -0700 (PDT) Received: from amir-ThinkPad-T480.lan ([5.29.249.86]) by smtp.gmail.com with ESMTPSA id v26-20020a05600c215a00b003eafc47eb09sm4333965wml.43.2023.03.18.03.15.39 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 18 Mar 2023 03:15:40 -0700 (PDT) From: Amir Goldstein To: Greg Kroah-Hartman Cc: Sasha Levin , "Darrick J . Wong" , Leah Rumancik , Chandan Babu R , Christian Brauner , linux-fsdevel@vger.kernel.org, linux-xfs@vger.kernel.org, stable@vger.kernel.org, Christoph Hellwig , Dave Chinner , Dave Chinner Subject: [PATCH 5.10 03/15] xfs: don't leak btree cursor when insrec fails after a split Date: Sat, 18 Mar 2023 12:15:17 +0200 Message-Id: <20230318101529.1361673-4-amir73il@gmail.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20230318101529.1361673-1-amir73il@gmail.com> References: <20230318101529.1361673-1-amir73il@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org From: "Darrick J. Wong" commit a54f78def73d847cb060b18c4e4a3d1d26c9ca6d upstream. The recent patch to improve btree cycle checking caused a regression when I rebased the in-memory btree branch atop the 5.19 for-next branch, because in-memory short-pointer btrees do not have AG numbers. This produced the following complaint from kmemleak: unreferenced object 0xffff88803d47dde8 (size 264): comm "xfs_io", pid 4889, jiffies 4294906764 (age 24.072s) hex dump (first 32 bytes): 90 4d 0b 0f 80 88 ff ff 00 a0 bd 05 80 88 ff ff .M.............. e0 44 3a a0 ff ff ff ff 00 df 08 06 80 88 ff ff .D:............. backtrace: [] xfbtree_dup_cursor+0x49/0xc0 [xfs] [] xfs_btree_dup_cursor+0x3b/0x200 [xfs] [] __xfs_btree_split+0x6ad/0x820 [xfs] [] xfs_btree_split+0x60/0x110 [xfs] [] xfs_btree_make_block_unfull+0x19a/0x1f0 [xfs] [] xfs_btree_insrec+0x3aa/0x810 [xfs] [] xfs_btree_insert+0xb3/0x240 [xfs] [] xfs_rmap_insert+0x99/0x200 [xfs] [] xfs_rmap_map_shared+0x192/0x5f0 [xfs] [] xfs_rmap_map_raw+0x6b/0x90 [xfs] [] xrep_rmap_stash+0xd5/0x1d0 [xfs] [] xrep_rmap_visit_bmbt+0xa0/0xf0 [xfs] [] xrep_rmap_scan_iext+0x56/0xa0 [xfs] [] xrep_rmap_scan_ifork+0xd8/0x160 [xfs] [] xrep_rmap_scan_inode+0x35/0x80 [xfs] [] xrep_rmap_find_rmaps+0x10e/0x270 [xfs] I noticed that xfs_btree_insrec has a bunch of debug code that return out of the function immediately, without freeing the "new" btree cursor that can be returned when _make_block_unfull calls xfs_btree_split. Fix the error return in this function to free the btree cursor. Signed-off-by: Darrick J. Wong Reviewed-by: Christoph Hellwig Reviewed-by: Dave Chinner Signed-off-by: Dave Chinner Signed-off-by: Amir Goldstein --- fs/xfs/libxfs/xfs_btree.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c index 24c7d30e41df..0926363179a7 100644 --- a/fs/xfs/libxfs/xfs_btree.c +++ b/fs/xfs/libxfs/xfs_btree.c @@ -3190,7 +3190,7 @@ xfs_btree_insrec( struct xfs_btree_block *block; /* btree block */ struct xfs_buf *bp; /* buffer for block */ union xfs_btree_ptr nptr; /* new block ptr */ - struct xfs_btree_cur *ncur; /* new btree cursor */ + struct xfs_btree_cur *ncur = NULL; /* new btree cursor */ union xfs_btree_key nkey; /* new block key */ union xfs_btree_key *lkey; int optr; /* old key/record index */ @@ -3270,7 +3270,7 @@ xfs_btree_insrec( #ifdef DEBUG error = xfs_btree_check_block(cur, block, level, bp); if (error) - return error; + goto error0; #endif /* @@ -3290,7 +3290,7 @@ xfs_btree_insrec( for (i = numrecs - ptr; i >= 0; i--) { error = xfs_btree_debug_check_ptr(cur, pp, i, level); if (error) - return error; + goto error0; } xfs_btree_shift_keys(cur, kp, 1, numrecs - ptr + 1); @@ -3375,6 +3375,8 @@ xfs_btree_insrec( return 0; error0: + if (ncur) + xfs_btree_del_cursor(ncur, error); return error; } From patchwork Sat Mar 18 10:15:18 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Amir Goldstein X-Patchwork-Id: 13179731 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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3D412C7618E for ; Sat, 18 Mar 2023 10:15:50 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229886AbjCRKPr (ORCPT ); Sat, 18 Mar 2023 06:15:47 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54816 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229878AbjCRKPp (ORCPT ); Sat, 18 Mar 2023 06:15:45 -0400 Received: from mail-wm1-x330.google.com (mail-wm1-x330.google.com [IPv6:2a00:1450:4864:20::330]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 01A9F28231; Sat, 18 Mar 2023 03:15:43 -0700 (PDT) Received: by mail-wm1-x330.google.com with SMTP id ip21-20020a05600ca69500b003ed56690948so4337522wmb.1; Sat, 18 Mar 2023 03:15:43 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1679134542; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=KgTIRGgLDwbCat3zch5MMu/wejmfIPRpBTIP2AxcCWU=; b=Dtf8yWt6UlzIXHWCMs+nXtlnl8nhPxxq5xr5iZoAhhgz92MuYOje4/BeaDkkrT9nGk HgxrKxO8D5oUw/YZU9kET/6VH618xtEsY5EYtnR0beR8j3Jgmi4l10ykI4X6fd1whkNK qN6Z38Z3/TPwNsPKe2EQtzvhtzZXRPRBhnc9dn0dh5p4rOUxpN90mhgmAtbHuY1G73I0 bmS8FuG17e53Gd2toQTruz4lhx+qARAbKxUIWTBtzeI4qABoVaT/J2lw+XkFp5W1DUH8 nUv/S1T5yCX55E4xIOHR2OKNLp/M8g/7DxneaEcFpsrXxIYoVx33VAZ01pGEwY0Ejil0 LK9Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1679134542; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=KgTIRGgLDwbCat3zch5MMu/wejmfIPRpBTIP2AxcCWU=; b=JeX7iV7cK8S2Xp9XyhhENRxzPSMBTy0/gPJ8zwtMvNRjIyCJvaHMmWbe37PwZlDY7t T3hERm87qF3G4wviCZ8L06mDWvaIa1/ROHNjLNHunaOjyLBY3EXsCJoLpYBLqyUsO++5 mQbu0fAELDGxo+3J9g0GtfgsHrnVlPXjWUM4ZLXfgl22J5ZYK07UWrLba+6GfR2DwthX hSfcLUw3q+kvrJJjO4vjSBSNMwM6+kieBKcuuF/ySbsG8eZ+YO8d63O+iNNhqcggb5GE CM9ip4BCGofhZSh2mfOnrYDwKyeBVYN4X8yw7lFz+yNdOtI8ISwpZVCgUUIdW+qdaOe6 DtmQ== X-Gm-Message-State: AO0yUKU+fQBXbxF+Ex5osfbuAcR4Kyyw8WDtiZHAjIfNejNh1i4dqdzG 0Fmklne8xOOqPLE+rRt17rg= X-Google-Smtp-Source: AK7set+RHOBxqbOwKyYtu03a9pBX3MMZm55ZYMXevQYuTkLuZcA3xe3Zl3mtGITIebUURvFci6hQZg== X-Received: by 2002:a05:600c:45d2:b0:3ed:2105:9abc with SMTP id s18-20020a05600c45d200b003ed21059abcmr20837891wmo.14.1679134542388; Sat, 18 Mar 2023 03:15:42 -0700 (PDT) Received: from amir-ThinkPad-T480.lan ([5.29.249.86]) by smtp.gmail.com with ESMTPSA id v26-20020a05600c215a00b003eafc47eb09sm4333965wml.43.2023.03.18.03.15.41 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 18 Mar 2023 03:15:42 -0700 (PDT) From: Amir Goldstein To: Greg Kroah-Hartman Cc: Sasha Levin , "Darrick J . Wong" , Leah Rumancik , Chandan Babu R , Christian Brauner , linux-fsdevel@vger.kernel.org, linux-xfs@vger.kernel.org, stable@vger.kernel.org, Dave Chinner Subject: [PATCH 5.10 04/15] xfs: remove XFS_PREALLOC_SYNC Date: Sat, 18 Mar 2023 12:15:18 +0200 Message-Id: <20230318101529.1361673-5-amir73il@gmail.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20230318101529.1361673-1-amir73il@gmail.com> References: <20230318101529.1361673-1-amir73il@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org From: Dave Chinner commit 472c6e46f589c26057596dcba160712a5b3e02c5 upstream. [partial backport for dependency - xfs_ioc_space() still uses XFS_PREALLOC_SYNC] Callers can acheive the same thing by calling xfs_log_force_inode() after making their modifications. There is no need for xfs_update_prealloc_flags() to do this. Signed-off-by: Dave Chinner Reviewed-by: Darrick J. Wong Signed-off-by: Darrick J. Wong Signed-off-by: Amir Goldstein --- fs/xfs/xfs_file.c | 13 +++++++------ fs/xfs/xfs_pnfs.c | 6 ++++-- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c index 4d6bf8d4974f..630525b1da77 100644 --- a/fs/xfs/xfs_file.c +++ b/fs/xfs/xfs_file.c @@ -94,8 +94,6 @@ xfs_update_prealloc_flags( ip->i_d.di_flags &= ~XFS_DIFLAG_PREALLOC; xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE); - if (flags & XFS_PREALLOC_SYNC) - xfs_trans_set_sync(tp); return xfs_trans_commit(tp); } @@ -1000,9 +998,6 @@ xfs_file_fallocate( } } - if (file->f_flags & O_DSYNC) - flags |= XFS_PREALLOC_SYNC; - error = xfs_update_prealloc_flags(ip, flags); if (error) goto out_unlock; @@ -1024,8 +1019,14 @@ xfs_file_fallocate( * leave shifted extents past EOF and hence losing access to * the data that is contained within them. */ - if (do_file_insert) + if (do_file_insert) { error = xfs_insert_file_space(ip, offset, len); + if (error) + goto out_unlock; + } + + if (file->f_flags & O_DSYNC) + error = xfs_log_force_inode(ip); out_unlock: xfs_iunlock(ip, iolock); diff --git a/fs/xfs/xfs_pnfs.c b/fs/xfs/xfs_pnfs.c index f3082a957d5e..64ab54f2fe81 100644 --- a/fs/xfs/xfs_pnfs.c +++ b/fs/xfs/xfs_pnfs.c @@ -164,10 +164,12 @@ xfs_fs_map_blocks( * that the blocks allocated and handed out to the client are * guaranteed to be present even after a server crash. */ - error = xfs_update_prealloc_flags(ip, - XFS_PREALLOC_SET | XFS_PREALLOC_SYNC); + error = xfs_update_prealloc_flags(ip, XFS_PREALLOC_SET); + if (!error) + error = xfs_log_force_inode(ip); if (error) goto out_unlock; + } else { xfs_iunlock(ip, lock_flags); } From patchwork Sat Mar 18 10:15:19 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Amir Goldstein X-Patchwork-Id: 13179732 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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1126EC76196 for ; Sat, 18 Mar 2023 10:15:51 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229923AbjCRKPs (ORCPT ); Sat, 18 Mar 2023 06:15:48 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54866 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229888AbjCRKPr (ORCPT ); Sat, 18 Mar 2023 06:15:47 -0400 Received: from mail-wm1-x32c.google.com (mail-wm1-x32c.google.com [IPv6:2a00:1450:4864:20::32c]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id BA1862410D; Sat, 18 Mar 2023 03:15:45 -0700 (PDT) Received: by mail-wm1-x32c.google.com with SMTP id g18so4702679wmk.0; Sat, 18 Mar 2023 03:15:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1679134544; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=SN211lgBBr1aJAJHF2diSYORjNp/PE39Z5mZHl1CgbA=; b=S42YikshP9VmCZNe8KEtdxt0mJtmNJoVS06CBk4TTsrOFAiLeo0dkBesF5awmrSrUE lV0lWT5+tUyeVTIY7njJh9K2yGDeUHIa2i5mZtlCfSjJduaq2/cVQ9g3eBrnwmx8kRlZ NXqRxOGo438w6G0qsytewNVd1Id1o2sCaZQLCzwTuuLTcT9HElOPTsD3WGyF8Ibq4aEQ kY2zycNJacFzoDOUDx9ttrmtER50L2XqJsClxm7YxceQdgo/aLsQtWj7XEsxfOMhmzLH v0RxZTcT3lxZOQotNKHPsbDUHsWFgWYUmfGNYin1PC5OXLtsEbVS4+RWBV4As8X/D0lQ wPrA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1679134544; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=SN211lgBBr1aJAJHF2diSYORjNp/PE39Z5mZHl1CgbA=; b=oWZ2MibVuiPg6/ekGiYIuNiRGNlmtZXEgvh0yo33FZl8lJP6e/F07/ZNaRRYcrlEwA gFv/bLIdDj5HMMnMgr9zbPQrmH0qmWXzt/SB6OXSLH5O0TdKnoa6U9HPXB/8OI8jtGmS oRSzxhzZP5P1qoi3GyVN8P7eHYWRZUp/9stb/H8HEgC/wY35gxCV+zpI2I0TzOB1M7Di GCUMEgXHcG9V9CaquuOEwBz4wDWHsctllhPBAzFsszqmODxrQ4GY8IKcV8Fg9vqWMYQ+ RmxrBPAWMD17L6EP3k57i0tg9BmKU41+OvB/wYmgk/iYuCc0ipbn1nCl4C+h9fZ2nMUQ lYsw== X-Gm-Message-State: AO0yUKXGmC/sdestQIHnVjAwxOyhmOSMJGgLGQ2F2xvxydSILjZWD3Gu PWeY8lsfw/1BAVczF8DnIaS4e7tNtbM= X-Google-Smtp-Source: AK7set8foq+c1VGq0VTPnmeHhsWdwegl+tE6RaFUNYIckxtZHbZP2AQ91385e5mX0szrEwWPf+Tmng== X-Received: by 2002:a7b:c3d7:0:b0:3ed:a80e:6dfa with SMTP id t23-20020a7bc3d7000000b003eda80e6dfamr1322285wmj.40.1679134543965; Sat, 18 Mar 2023 03:15:43 -0700 (PDT) Received: from amir-ThinkPad-T480.lan ([5.29.249.86]) by smtp.gmail.com with ESMTPSA id v26-20020a05600c215a00b003eafc47eb09sm4333965wml.43.2023.03.18.03.15.42 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 18 Mar 2023 03:15:43 -0700 (PDT) From: Amir Goldstein To: Greg Kroah-Hartman Cc: Sasha Levin , "Darrick J . Wong" , Leah Rumancik , Chandan Babu R , Christian Brauner , linux-fsdevel@vger.kernel.org, linux-xfs@vger.kernel.org, stable@vger.kernel.org, Dave Chinner Subject: [PATCH 5.10 05/15] xfs: fallocate() should call file_modified() Date: Sat, 18 Mar 2023 12:15:19 +0200 Message-Id: <20230318101529.1361673-6-amir73il@gmail.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20230318101529.1361673-1-amir73il@gmail.com> References: <20230318101529.1361673-1-amir73il@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org From: Dave Chinner commit fbe7e520036583a783b13ff9744e35c2a329d9a4 upstream. In XFS, we always update the inode change and modification time when any fallocate() operation succeeds. Furthermore, as various fallocate modes can change the file contents (extending EOF, punching holes, zeroing things, shifting extents), we should drop file privileges like suid just like we do for a regular write(). There's already a VFS helper that figures all this out for us, so use that. The net effect of this is that we no longer drop suid/sgid if the caller is root, but we also now drop file capabilities. We also move the xfs_update_prealloc_flags() function so that it now is only called by the scope that needs to set the the prealloc flag. Based on a patch from Darrick Wong. Signed-off-by: Dave Chinner Reviewed-by: Darrick J. Wong Signed-off-by: Darrick J. Wong Signed-off-by: Amir Goldstein --- fs/xfs/xfs_file.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c index 630525b1da77..a95af57a59a7 100644 --- a/fs/xfs/xfs_file.c +++ b/fs/xfs/xfs_file.c @@ -895,6 +895,10 @@ xfs_file_fallocate( goto out_unlock; } + error = file_modified(file); + if (error) + goto out_unlock; + if (mode & FALLOC_FL_PUNCH_HOLE) { error = xfs_free_file_space(ip, offset, len); if (error) @@ -996,11 +1000,12 @@ xfs_file_fallocate( if (error) goto out_unlock; } - } - error = xfs_update_prealloc_flags(ip, flags); - if (error) - goto out_unlock; + error = xfs_update_prealloc_flags(ip, XFS_PREALLOC_SET); + if (error) + goto out_unlock; + + } /* Change file size if needed */ if (new_size) { From patchwork Sat Mar 18 10:15:20 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Amir Goldstein X-Patchwork-Id: 13179733 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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 66623C61DA4 for ; Sat, 18 Mar 2023 10:16:00 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229958AbjCRKP6 (ORCPT ); Sat, 18 Mar 2023 06:15:58 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54914 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229904AbjCRKPr (ORCPT ); Sat, 18 Mar 2023 06:15:47 -0400 Received: from mail-wm1-x330.google.com (mail-wm1-x330.google.com [IPv6:2a00:1450:4864:20::330]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 2A0C62E838; Sat, 18 Mar 2023 03:15:46 -0700 (PDT) Received: by mail-wm1-x330.google.com with SMTP id ip21-20020a05600ca69500b003ed56690948so4337563wmb.1; Sat, 18 Mar 2023 03:15:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1679134545; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=2kUvv3uBGGOzJwwn6KRNuDlAWVNfTAIS5jkw/tF5MpM=; b=eW37GhIIeMcGMB3jYcOgMYx9qZZoIQkvsQHcLvSPYsyeSPxcfdo0KMKFVMT/Su4y5U drmiUmYB3lZU7NA6ECeQa6Gf9I47n2TPISb2yOB3WWLqu43RJkCcD5zOz4i5UzskRGPE 1gLM6o4sKicFAGOYBINFv23VflvDYi+Mv7ud/dmvoWVgCRVx0f0mQEL2jdo1ZGMKIKmR MCNr2rzrKpvSJPNNdw5osyJr53Rbt1HeUVm5pD8i8MP+k6WPC1/c7gw3phk+CH8PVGW8 B/+JZsHau+rGVGK+LM5KB+fokxFJZWSfclKvln5fd1wutPYdY+afd1VOLXrJcbPgh31u S6fg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1679134545; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=2kUvv3uBGGOzJwwn6KRNuDlAWVNfTAIS5jkw/tF5MpM=; b=a/AeDjfAX8qshX7DarkSSfGSTKHbMw/oS95jx2OvF6rc/V1bBIG1R9yRUijKSstptC PCrdCZwxyI3s1DXd4VWruz3hL6QK0h1mwX4t4X8hQ4caLJTc+mxrXgW/HEEPlGnGWWJs ZVBSIwztyjbnxQ/ia6unlrlc61ZJoHYEvWxZgNkDyTcujlVRxrdJwgeU8+Lcggv2yoL/ IQkZeQpUtpOUnrR9GdHJ+5YoYW9cz95tHlwKYft0BqCa+hdrLL+d/Um8lez+Kxw2oq5a sggtmWN8Bi4Dj2nRLcE7KMoB9cGGKrj4KrSBCHj6zOAy3GzBu645F7U4/89qPdmDTEt4 JL3g== X-Gm-Message-State: AO0yUKU2c/QCnGuttlTFS8i6blFc7wuctHqmOY/9oX9fHP4aeFbY/DTG +Po61B8mFp04W7JKWhfuJ9U= X-Google-Smtp-Source: AK7set+mr5Mxf3uSiKYQFGIidIrZ2inP8iYGI7qQtyPDygLenp7ZDeJMAxVl6M/5e/HPtowVHAHcKQ== X-Received: by 2002:a05:600c:3b99:b0:3ed:2352:eebd with SMTP id n25-20020a05600c3b9900b003ed2352eebdmr20339096wms.11.1679134545492; Sat, 18 Mar 2023 03:15:45 -0700 (PDT) Received: from amir-ThinkPad-T480.lan ([5.29.249.86]) by smtp.gmail.com with ESMTPSA id v26-20020a05600c215a00b003eafc47eb09sm4333965wml.43.2023.03.18.03.15.44 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 18 Mar 2023 03:15:45 -0700 (PDT) From: Amir Goldstein To: Greg Kroah-Hartman Cc: Sasha Levin , "Darrick J . Wong" , Leah Rumancik , Chandan Babu R , Christian Brauner , linux-fsdevel@vger.kernel.org, linux-xfs@vger.kernel.org, stable@vger.kernel.org, Dave Chinner Subject: [PATCH 5.10 06/15] xfs: set prealloc flag in xfs_alloc_file_space() Date: Sat, 18 Mar 2023 12:15:20 +0200 Message-Id: <20230318101529.1361673-7-amir73il@gmail.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20230318101529.1361673-1-amir73il@gmail.com> References: <20230318101529.1361673-1-amir73il@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org From: Dave Chinner commit 0b02c8c0d75a738c98c35f02efb36217c170d78c upstream. [backport for 5.10.y] Now that we only call xfs_update_prealloc_flags() from xfs_file_fallocate() in the case where we need to set the preallocation flag, do this in xfs_alloc_file_space() where we already have the inode joined into a transaction and get rid of the call to xfs_update_prealloc_flags() from the fallocate code. This also means that we now correctly avoid setting the XFS_DIFLAG_PREALLOC flag when xfs_is_always_cow_inode() is true, as these inodes will never have preallocated extents. Signed-off-by: Dave Chinner Reviewed-by: Darrick J. Wong Signed-off-by: Darrick J. Wong Signed-off-by: Amir Goldstein --- fs/xfs/xfs_bmap_util.c | 9 +++------ fs/xfs/xfs_file.c | 8 -------- 2 files changed, 3 insertions(+), 14 deletions(-) diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c index 7371a7f7c652..fbab1042bc90 100644 --- a/fs/xfs/xfs_bmap_util.c +++ b/fs/xfs/xfs_bmap_util.c @@ -800,9 +800,6 @@ xfs_alloc_file_space( quota_flag = XFS_QMOPT_RES_REGBLKS; } - /* - * Allocate and setup the transaction. - */ error = xfs_trans_alloc(mp, &M_RES(mp)->tr_write, resblks, resrtextents, 0, &tp); @@ -830,9 +827,9 @@ xfs_alloc_file_space( if (error) goto error0; - /* - * Complete the transaction - */ + ip->i_d.di_flags |= XFS_DIFLAG_PREALLOC; + xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE); + error = xfs_trans_commit(tp); xfs_iunlock(ip, XFS_ILOCK_EXCL); if (error) diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c index a95af57a59a7..9b6c5ba5fdfb 100644 --- a/fs/xfs/xfs_file.c +++ b/fs/xfs/xfs_file.c @@ -850,7 +850,6 @@ xfs_file_fallocate( struct inode *inode = file_inode(file); struct xfs_inode *ip = XFS_I(inode); long error; - enum xfs_prealloc_flags flags = 0; uint iolock = XFS_IOLOCK_EXCL | XFS_MMAPLOCK_EXCL; loff_t new_size = 0; bool do_file_insert = false; @@ -948,8 +947,6 @@ xfs_file_fallocate( } do_file_insert = true; } else { - flags |= XFS_PREALLOC_SET; - if (!(mode & FALLOC_FL_KEEP_SIZE) && offset + len > i_size_read(inode)) { new_size = offset + len; @@ -1000,11 +997,6 @@ xfs_file_fallocate( if (error) goto out_unlock; } - - error = xfs_update_prealloc_flags(ip, XFS_PREALLOC_SET); - if (error) - goto out_unlock; - } /* Change file size if needed */ From patchwork Sat Mar 18 10:15:21 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Amir Goldstein X-Patchwork-Id: 13179734 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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id CFD49C761A6 for ; Sat, 18 Mar 2023 10:16:00 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229904AbjCRKP6 (ORCPT ); Sat, 18 Mar 2023 06:15:58 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54998 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229888AbjCRKPu (ORCPT ); Sat, 18 Mar 2023 06:15:50 -0400 Received: from mail-wm1-x331.google.com (mail-wm1-x331.google.com [IPv6:2a00:1450:4864:20::331]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D8DFC32E42; Sat, 18 Mar 2023 03:15:48 -0700 (PDT) Received: by mail-wm1-x331.google.com with SMTP id p13-20020a05600c358d00b003ed346d4522so4726761wmq.2; Sat, 18 Mar 2023 03:15:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1679134547; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=X0umKp2XY96WqbVyuc0DfJHbI4fSMf6LKhYhZe2jZwM=; b=Jk54qC3ZazmMXpbT/HgytgYn/sYzgt7ksawjYYt5A1EPsmfOkHbqX4BLYU/kKeVSil lxjtN6v7W85HoP0vi3Jj8jeJ3GtrM6ZFXSDH6lFHYqkAi+ccqvyTGqJe5cTzSd6A3oVt 4gcJkZ0ADqfdO3NKoFnnMRWxPmsUcZE+VlZOTITpmMEY9wxy5kYJ/N91zOCHwO0wQoEj A6du2Nfw5tj0coYjfq74ggEVgpRsJlYJsWrI8DiJg5TIgVRhseJmfOnjmqBV7wggSEy9 ItxBj02R6kdHN208u8r+bCIIwpnooK6l9X1S/nnlzw6Aebym1yO1OyI2xNhcdvIiTdJq v2mw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1679134547; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=X0umKp2XY96WqbVyuc0DfJHbI4fSMf6LKhYhZe2jZwM=; b=rpZx881rYRhBpyQ0+nt4tQO8T5lB3iXu4Y5pXEZIKYOB7uC/Gbi2PrvWrHdMjzPFMQ g20pqLd48VtOnkIazSS+TTqoNg1EJMQcxdQj6Tu8QeBwZ8HeLhsemnC5pOQJZK5MrGs0 fZAVt7EETBUuqC3v49ZMjIfgcd5lcp51y4JGBrdlrhrqbMuymxZdsgIK/vSse+KLM0+e eNPTTTmwowkV0nFAv2tM+ORz4/B9q4LwpekO5fx1V07g45bKhwE/uD8zAFsxK4x05m6y QtrrgM62G3aa/IymAVdbl0AFAf82g/maabMUV0sC1i6cLm4suvTCc9oiT2H4WDTmcOLi splA== X-Gm-Message-State: AO0yUKUc2mTffeQvC+yeIx6rIoGTFhWwa1mMP2T5+vb4kznYftXlrXWd Q4sfdMha1zMf2S8wCk65tVw= X-Google-Smtp-Source: AK7set+ojK3WGHxMuXp8HUXrv1qkb1nxhdgb+GokuopgjPtKa1oip7V0fdhJTPcLNNcYhcqHCszOjA== X-Received: by 2002:a05:600c:4453:b0:3eb:389d:156c with SMTP id v19-20020a05600c445300b003eb389d156cmr28586169wmn.37.1679134547213; Sat, 18 Mar 2023 03:15:47 -0700 (PDT) Received: from amir-ThinkPad-T480.lan ([5.29.249.86]) by smtp.gmail.com with ESMTPSA id v26-20020a05600c215a00b003eafc47eb09sm4333965wml.43.2023.03.18.03.15.45 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 18 Mar 2023 03:15:46 -0700 (PDT) From: Amir Goldstein To: Greg Kroah-Hartman Cc: Sasha Levin , "Darrick J . Wong" , Leah Rumancik , Chandan Babu R , Christian Brauner , linux-fsdevel@vger.kernel.org, linux-xfs@vger.kernel.org, stable@vger.kernel.org, Dave Chinner , Christoph Hellwig Subject: [PATCH 5.10 07/15] xfs: use setattr_copy to set vfs inode attributes Date: Sat, 18 Mar 2023 12:15:21 +0200 Message-Id: <20230318101529.1361673-8-amir73il@gmail.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20230318101529.1361673-1-amir73il@gmail.com> References: <20230318101529.1361673-1-amir73il@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org From: "Darrick J. Wong" commit e014f37db1a2d109afa750042ac4d69cf3e3d88e upstream. [remove userns argument of setattr_copy() for 5.10.y backport] Filipe Manana pointed out that XFS' behavior w.r.t. setuid/setgid revocation isn't consistent with btrfs[1] or ext4. Those two filesystems use the VFS function setattr_copy to convey certain attributes from struct iattr into the VFS inode structure. Andrey Zhadchenko reported[2] that XFS uses the wrong user namespace to decide if it should clear setgid and setuid on a file attribute update. This is a second symptom of the problem that Filipe noticed. XFS, on the other hand, open-codes setattr_copy in xfs_setattr_mode, xfs_setattr_nonsize, and xfs_setattr_time. Regrettably, setattr_copy is /not/ a simple copy function; it contains additional logic to clear the setgid bit when setting the mode, and XFS' version no longer matches. The VFS implements its own setuid/setgid stripping logic, which establishes consistent behavior. It's a tad unfortunate that it's scattered across notify_change, should_remove_suid, and setattr_copy but XFS should really follow the Linux VFS. Adapt XFS to use the VFS functions and get rid of the old functions. [1] https://lore.kernel.org/fstests/CAL3q7H47iNQ=Wmk83WcGB-KBJVOEtR9+qGczzCeXJ9Y2KCV25Q@mail.gmail.com/ [2] https://lore.kernel.org/linux-xfs/20220221182218.748084-1-andrey.zhadchenko@virtuozzo.com/ Fixes: 7fa294c8991c ("userns: Allow chown and setgid preservation") Signed-off-by: Darrick J. Wong Reviewed-by: Dave Chinner Reviewed-by: Christoph Hellwig Reviewed-by: Christian Brauner Signed-off-by: Amir Goldstein --- fs/xfs/xfs_iops.c | 56 +++-------------------------------------------- fs/xfs/xfs_pnfs.c | 3 ++- 2 files changed, 5 insertions(+), 54 deletions(-) diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c index 6a3026e78a9b..69fef29df428 100644 --- a/fs/xfs/xfs_iops.c +++ b/fs/xfs/xfs_iops.c @@ -595,37 +595,6 @@ xfs_vn_getattr( return 0; } -static void -xfs_setattr_mode( - struct xfs_inode *ip, - struct iattr *iattr) -{ - struct inode *inode = VFS_I(ip); - umode_t mode = iattr->ia_mode; - - ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL)); - - inode->i_mode &= S_IFMT; - inode->i_mode |= mode & ~S_IFMT; -} - -void -xfs_setattr_time( - struct xfs_inode *ip, - struct iattr *iattr) -{ - struct inode *inode = VFS_I(ip); - - ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL)); - - if (iattr->ia_valid & ATTR_ATIME) - inode->i_atime = iattr->ia_atime; - if (iattr->ia_valid & ATTR_CTIME) - inode->i_ctime = iattr->ia_ctime; - if (iattr->ia_valid & ATTR_MTIME) - inode->i_mtime = iattr->ia_mtime; -} - static int xfs_vn_change_ok( struct dentry *dentry, @@ -740,16 +709,6 @@ xfs_setattr_nonsize( goto out_cancel; } - /* - * CAP_FSETID overrides the following restrictions: - * - * The set-user-ID and set-group-ID bits of a file will be - * cleared upon successful return from chown() - */ - if ((inode->i_mode & (S_ISUID|S_ISGID)) && - !capable(CAP_FSETID)) - inode->i_mode &= ~(S_ISUID|S_ISGID); - /* * Change the ownerships and register quota modifications * in the transaction. @@ -761,7 +720,6 @@ xfs_setattr_nonsize( olddquot1 = xfs_qm_vop_chown(tp, ip, &ip->i_udquot, udqp); } - inode->i_uid = uid; } if (!gid_eq(igid, gid)) { if (XFS_IS_QUOTA_RUNNING(mp) && XFS_IS_GQUOTA_ON(mp)) { @@ -772,15 +730,10 @@ xfs_setattr_nonsize( olddquot2 = xfs_qm_vop_chown(tp, ip, &ip->i_gdquot, gdqp); } - inode->i_gid = gid; } } - if (mask & ATTR_MODE) - xfs_setattr_mode(ip, iattr); - if (mask & (ATTR_ATIME|ATTR_CTIME|ATTR_MTIME)) - xfs_setattr_time(ip, iattr); - + setattr_copy(inode, iattr); xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE); XFS_STATS_INC(mp, xs_ig_attrchg); @@ -1025,11 +978,8 @@ xfs_setattr_size( xfs_inode_clear_eofblocks_tag(ip); } - if (iattr->ia_valid & ATTR_MODE) - xfs_setattr_mode(ip, iattr); - if (iattr->ia_valid & (ATTR_ATIME|ATTR_CTIME|ATTR_MTIME)) - xfs_setattr_time(ip, iattr); - + ASSERT(!(iattr->ia_valid & (ATTR_UID | ATTR_GID))); + setattr_copy(inode, iattr); xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE); XFS_STATS_INC(mp, xs_ig_attrchg); diff --git a/fs/xfs/xfs_pnfs.c b/fs/xfs/xfs_pnfs.c index 64ab54f2fe81..053b99929f83 100644 --- a/fs/xfs/xfs_pnfs.c +++ b/fs/xfs/xfs_pnfs.c @@ -285,7 +285,8 @@ xfs_fs_commit_blocks( xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL); xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE); - xfs_setattr_time(ip, iattr); + ASSERT(!(iattr->ia_valid & (ATTR_UID | ATTR_GID))); + setattr_copy(inode, iattr); if (update_isize) { i_size_write(inode, iattr->ia_size); ip->i_d.di_size = iattr->ia_size; From patchwork Sat Mar 18 10:15:22 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Amir Goldstein X-Patchwork-Id: 13179735 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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 425A8C74A5B for ; Sat, 18 Mar 2023 10:16:09 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229945AbjCRKQH (ORCPT ); Sat, 18 Mar 2023 06:16:07 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55194 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229845AbjCRKP5 (ORCPT ); Sat, 18 Mar 2023 06:15:57 -0400 Received: from mail-wm1-x32a.google.com (mail-wm1-x32a.google.com [IPv6:2a00:1450:4864:20::32a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id BB5353CE1E; Sat, 18 Mar 2023 03:15:50 -0700 (PDT) Received: by mail-wm1-x32a.google.com with SMTP id r19-20020a05600c459300b003eb3e2a5e7bso4731490wmo.0; Sat, 18 Mar 2023 03:15:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1679134549; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=iF1mN6zvABPJFTbU5QEGrkBoU8KKs2uGJMLFx2CtOkE=; b=heAivPpDbsDpsG3pYYLVPIP3jNFjWdOgWWUPJGZTaugqG3XvhxV1H2RO4iZR9y7EOD gdqnFcUC0qluSkvfNP1oGY7GkFEKAxrhDzidFZENmq48zQ2rY5lC8ZwhuOk2E03RwYKp ADPOB9ZBqpZmAvXLMjomfb1Hs5ut+Oxnx4j10BOO/CgAzBz42sOvhBUrt+DMfvmfkUiK d7nRR8rxFeoj7fmobR/uxgdctnW9ZAVetAubOKuWCRfQcHDBAIaKBTrk89iE+bdjibtg YVrcO+TuzabSgk+yU/q9+pu1KyscLa5CQE9yvNUbR07FFa2hFO+BqvBg2v6ay/zJyRWO maxQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1679134549; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=iF1mN6zvABPJFTbU5QEGrkBoU8KKs2uGJMLFx2CtOkE=; b=NB0ubepkuHDXKgJxkwi2y31obZ8aCBpJIM4RSrgyXtej/NWwcILjRI01Ku/iCFg0JT WeGHVbmfWlAkq4nwjOf9KOD6Uz9n1sYc3oFaQIeW4QWu+d0wUGa1Dps88lNTdfRNOF7z 6iCa5JBU7s5UC6U2N3RNMk0RG3Dpfx6CA1kHnLxAEgiPWddRw1cgRRO7x2qOKpjBFQKR 1eTYlalBoRlDPMTT5dPVJz7TEX1Cfw9NuFaw0MebrR3qDrpqLemNklJxn+dRSP2UoBdG RL6Qh3z/V3gd4/tGiyQ5pKe6mXBcTu6i2AbTonsJ7ion+2q/uVU1v7sjlt/Hiv65V1Wc r4oA== X-Gm-Message-State: AO0yUKVh2ahNhc6XFEFqY1pH4aq3HoQcG6VY+Es1b8HOGGrHpL733Q8D 4y7HnphDkSt6Z8bHf/o8c7s= X-Google-Smtp-Source: AK7set/L2wQAz3z1r+j/ukF8WJ3qi+NCSFrwZhS1ouaaZJTRQieKNOFbI0mH/A6XJ+4NWCAzPYbf/A== X-Received: by 2002:a05:600c:28c:b0:3ed:5d41:f964 with SMTP id 12-20020a05600c028c00b003ed5d41f964mr4159853wmk.1.1679134548955; Sat, 18 Mar 2023 03:15:48 -0700 (PDT) Received: from amir-ThinkPad-T480.lan ([5.29.249.86]) by smtp.gmail.com with ESMTPSA id v26-20020a05600c215a00b003eafc47eb09sm4333965wml.43.2023.03.18.03.15.47 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 18 Mar 2023 03:15:48 -0700 (PDT) From: Amir Goldstein To: Greg Kroah-Hartman Cc: Sasha Levin , "Darrick J . Wong" , Leah Rumancik , Chandan Babu R , Christian Brauner , linux-fsdevel@vger.kernel.org, linux-xfs@vger.kernel.org, stable@vger.kernel.org, Yang Xu , Jeff Layton Subject: [PATCH 5.10 08/15] fs: add mode_strip_sgid() helper Date: Sat, 18 Mar 2023 12:15:22 +0200 Message-Id: <20230318101529.1361673-9-amir73il@gmail.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20230318101529.1361673-1-amir73il@gmail.com> References: <20230318101529.1361673-1-amir73il@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org From: Yang Xu commit 2b3416ceff5e6bd4922f6d1c61fb68113dd82302 upstream. [remove userns argument of helper for 5.10.y backport] Add a dedicated helper to handle the setgid bit when creating a new file in a setgid directory. This is a preparatory patch for moving setgid stripping into the vfs. The patch contains no functional changes. Currently the setgid stripping logic is open-coded directly in inode_init_owner() and the individual filesystems are responsible for handling setgid inheritance. Since this has proven to be brittle as evidenced by old issues we uncovered over the last months (see [1] to [3] below) we will try to move this logic into the vfs. Link: e014f37db1a2 ("xfs: use setattr_copy to set vfs inode attributes") [1] Link: 01ea173e103e ("xfs: fix up non-directory creation in SGID directories") [2] Link: fd84bfdddd16 ("ceph: fix up non-directory creation in SGID directories") [3] Link: https://lore.kernel.org/r/1657779088-2242-1-git-send-email-xuyang2018.jy@fujitsu.com Reviewed-by: Darrick J. Wong Reviewed-by: Christian Brauner (Microsoft) Reviewed-and-Tested-by: Jeff Layton Signed-off-by: Yang Xu Signed-off-by: Christian Brauner (Microsoft) Signed-off-by: Amir Goldstein --- fs/inode.c | 34 ++++++++++++++++++++++++++++++---- include/linux/fs.h | 1 + 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/fs/inode.c b/fs/inode.c index 9f49e0bdc2f7..23d03abcb0ff 100644 --- a/fs/inode.c +++ b/fs/inode.c @@ -2147,10 +2147,8 @@ void inode_init_owner(struct inode *inode, const struct inode *dir, /* Directories are special, and always inherit S_ISGID */ if (S_ISDIR(mode)) mode |= S_ISGID; - else if ((mode & (S_ISGID | S_IXGRP)) == (S_ISGID | S_IXGRP) && - !in_group_p(inode->i_gid) && - !capable_wrt_inode_uidgid(dir, CAP_FSETID)) - mode &= ~S_ISGID; + else + mode = mode_strip_sgid(dir, mode); } else inode->i_gid = current_fsgid(); inode->i_mode = mode; @@ -2382,3 +2380,31 @@ int vfs_ioc_fssetxattr_check(struct inode *inode, const struct fsxattr *old_fa, return 0; } EXPORT_SYMBOL(vfs_ioc_fssetxattr_check); + +/** + * mode_strip_sgid - handle the sgid bit for non-directories + * @dir: parent directory inode + * @mode: mode of the file to be created in @dir + * + * If the @mode of the new file has both the S_ISGID and S_IXGRP bit + * raised and @dir has the S_ISGID bit raised ensure that the caller is + * either in the group of the parent directory or they have CAP_FSETID + * in their user namespace and are privileged over the parent directory. + * In all other cases, strip the S_ISGID bit from @mode. + * + * Return: the new mode to use for the file + */ +umode_t mode_strip_sgid(const struct inode *dir, umode_t mode) +{ + if ((mode & (S_ISGID | S_IXGRP)) != (S_ISGID | S_IXGRP)) + return mode; + if (S_ISDIR(mode) || !dir || !(dir->i_mode & S_ISGID)) + return mode; + if (in_group_p(dir->i_gid)) + return mode; + if (capable_wrt_inode_uidgid(dir, CAP_FSETID)) + return mode; + + return mode & ~S_ISGID; +} +EXPORT_SYMBOL(mode_strip_sgid); diff --git a/include/linux/fs.h b/include/linux/fs.h index 74e19bccbf73..527791e4860b 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -1768,6 +1768,7 @@ extern long compat_ptr_ioctl(struct file *file, unsigned int cmd, extern void inode_init_owner(struct inode *inode, const struct inode *dir, umode_t mode); extern bool may_open_dev(const struct path *path); +umode_t mode_strip_sgid(const struct inode *dir, umode_t mode); /* * This is the "filldir" function type, used by readdir() to let From patchwork Sat Mar 18 10:15:23 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Amir Goldstein X-Patchwork-Id: 13179737 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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id C2B97C76196 for ; Sat, 18 Mar 2023 10:16:09 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229985AbjCRKQI (ORCPT ); Sat, 18 Mar 2023 06:16:08 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55200 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229943AbjCRKP6 (ORCPT ); Sat, 18 Mar 2023 06:15:58 -0400 Received: from mail-wm1-x32e.google.com (mail-wm1-x32e.google.com [IPv6:2a00:1450:4864:20::32e]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 89BE93D927; Sat, 18 Mar 2023 03:15:51 -0700 (PDT) Received: by mail-wm1-x32e.google.com with SMTP id bh21-20020a05600c3d1500b003ed1ff06fb0so4717762wmb.3; Sat, 18 Mar 2023 03:15:51 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1679134551; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=nfqqH40w4HE6WzfQRiTia8TiMis5qYAinCOETOm8WDM=; b=KAsqxX7FSI9OQmhMJzba26dg/uFT4HGDsxRL5fIwlTzD1DTIbhHNDY0kVNF4Nn0oZB 8irUvzrf3rJ8miQir1as0rCbb8/x+Y4wwqfkY4BW901u4iSCrlkpvDZJ83w3Qica8EsX 1ENR3KZa8JnWwOlAkHiU9wMbzLg2PFi1ibjUV5kOkkGubYpbf8B3P0ZuPMg/LWHxpZvY ui/VVX9hIh6C6xeoU/MgHgywYp4sS66XHD1Lg/BqIR03go6gn0byFIG8DU2DE70VnkJw d1te6Bbs1nERQg01QDXFNSs4cDhGh3eNKcb9wVr1HSLTkSOuWk1rxA0JnB5jxrZ0TUcF 3d/g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1679134551; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=nfqqH40w4HE6WzfQRiTia8TiMis5qYAinCOETOm8WDM=; b=fUzQno2rlrWpwsqKqWVoiMRfI0tLVXgprDAzYFjRX7RtK8Gu9lwrCr66BNiMoEdpLX sEEIcNLx/tisOwA573SjSyXMLqiXj26Pfcncgv7BOwAefxDbajtRHKPN9L9iK5JKo7RH oraue06g+zhvw5DtcgB3/NxM4JrjYicZQiuyhIjyTCznMWYdEKr6D+a1ZscS7R8Yhdab 31wgRWoH596LTZgPh3O0cMonQGzIihQxwKnsVwlzAMD23JD/42tPU30M5MCy1ypTKGVf 3YcHvOR+Ro7RtESln4XacZwZYE2N4vCjbaXFzbGhlMsK+FW7qnY08JfDrUp5lm3XlEs1 lLRw== X-Gm-Message-State: AO0yUKUurQrqA4C9k2JaC7URfzqZTXvvxjLxQ4ZLq/thzwmRmv38BkXe HVBzzrUfx+kHZYUPcXGPFN0= X-Google-Smtp-Source: AK7set87jYH6NPigxChBc9092wYEk+8CLhT3hMCFDf77vKOsgqfmdEz6KtcUBkv40A0bQzg0vWZXaA== X-Received: by 2002:a05:600c:4506:b0:3ed:253c:621b with SMTP id t6-20020a05600c450600b003ed253c621bmr21269706wmo.21.1679134550781; Sat, 18 Mar 2023 03:15:50 -0700 (PDT) Received: from amir-ThinkPad-T480.lan ([5.29.249.86]) by smtp.gmail.com with ESMTPSA id v26-20020a05600c215a00b003eafc47eb09sm4333965wml.43.2023.03.18.03.15.49 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 18 Mar 2023 03:15:50 -0700 (PDT) From: Amir Goldstein To: Greg Kroah-Hartman Cc: Sasha Levin , "Darrick J . Wong" , Leah Rumancik , Chandan Babu R , Christian Brauner , linux-fsdevel@vger.kernel.org, linux-xfs@vger.kernel.org, stable@vger.kernel.org, Yang Xu , Dave Chinner , Jeff Layton Subject: [PATCH 5.10 09/15] fs: move S_ISGID stripping into the vfs_*() helpers Date: Sat, 18 Mar 2023 12:15:23 +0200 Message-Id: <20230318101529.1361673-10-amir73il@gmail.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20230318101529.1361673-1-amir73il@gmail.com> References: <20230318101529.1361673-1-amir73il@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org From: Yang Xu commit 1639a49ccdce58ea248841ed9b23babcce6dbb0b upstream. [remove userns argument of helpers for 5.10.y backport] Move setgid handling out of individual filesystems and into the VFS itself to stop the proliferation of setgid inheritance bugs. Creating files that have both the S_IXGRP and S_ISGID bit raised in directories that themselves have the S_ISGID bit set requires additional privileges to avoid security issues. When a filesystem creates a new inode it needs to take care that the caller is either in the group of the newly created inode or they have CAP_FSETID in their current user namespace and are privileged over the parent directory of the new inode. If any of these two conditions is true then the S_ISGID bit can be raised for an S_IXGRP file and if not it needs to be stripped. However, there are several key issues with the current implementation: * S_ISGID stripping logic is entangled with umask stripping. If a filesystem doesn't support or enable POSIX ACLs then umask stripping is done directly in the vfs before calling into the filesystem. If the filesystem does support POSIX ACLs then unmask stripping may be done in the filesystem itself when calling posix_acl_create(). Since umask stripping has an effect on S_ISGID inheritance, e.g., by stripping the S_IXGRP bit from the file to be created and all relevant filesystems have to call posix_acl_create() before inode_init_owner() where we currently take care of S_ISGID handling S_ISGID handling is order dependent. IOW, whether or not you get a setgid bit depends on POSIX ACLs and umask and in what order they are called. Note that technically filesystems are free to impose their own ordering between posix_acl_create() and inode_init_owner() meaning that there's additional ordering issues that influence S_SIGID inheritance. * Filesystems that don't rely on inode_init_owner() don't get S_ISGID stripping logic. While that may be intentional (e.g. network filesystems might just defer setgid stripping to a server) it is often just a security issue. This is not just ugly it's unsustainably messy especially since we do still have bugs in this area years after the initial round of setgid bugfixes. So the current state is quite messy and while we won't be able to make it completely clean as posix_acl_create() is still a filesystem specific call we can improve the S_SIGD stripping situation quite a bit by hoisting it out of inode_init_owner() and into the vfs creation operations. This means we alleviate the burden for filesystems to handle S_ISGID stripping correctly and can standardize the ordering between S_ISGID and umask stripping in the vfs. We add a new helper vfs_prepare_mode() so S_ISGID handling is now done in the VFS before umask handling. This has S_ISGID handling is unaffected unaffected by whether umask stripping is done by the VFS itself (if no POSIX ACLs are supported or enabled) or in the filesystem in posix_acl_create() (if POSIX ACLs are supported). The vfs_prepare_mode() helper is called directly in vfs_*() helpers that create new filesystem objects. We need to move them into there to make sure that filesystems like overlayfs hat have callchains like: sys_mknod() -> do_mknodat(mode) -> .mknod = ovl_mknod(mode) -> ovl_create(mode) -> vfs_mknod(mode) get S_ISGID stripping done when calling into lower filesystems via vfs_*() creation helpers. Moving vfs_prepare_mode() into e.g. vfs_mknod() takes care of that. This is in any case semantically cleaner because S_ISGID stripping is VFS security requirement. Security hooks so far have seen the mode with the umask applied but without S_ISGID handling done. The relevant hooks are called outside of vfs_*() creation helpers so by calling vfs_prepare_mode() from vfs_*() helpers the security hooks would now see the mode without umask stripping applied. For now we fix this by passing the mode with umask settings applied to not risk any regressions for LSM hooks. IOW, nothing changes for LSM hooks. It is worth pointing out that security hooks never saw the mode that is seen by the filesystem when actually creating the file. They have always been completely misplaced for that to work. The following filesystems use inode_init_owner() and thus relied on S_ISGID stripping: spufs, 9p, bfs, btrfs, ext2, ext4, f2fs, hfsplus, hugetlbfs, jfs, minix, nilfs2, ntfs3, ocfs2, omfs, overlayfs, ramfs, reiserfs, sysv, ubifs, udf, ufs, xfs, zonefs, bpf, tmpfs. All of the above filesystems end up calling inode_init_owner() when new filesystem objects are created through the ->mkdir(), ->mknod(), ->create(), ->tmpfile(), ->rename() inode operations. Since directories always inherit the S_ISGID bit with the exception of xfs when irix_sgid_inherit mode is turned on S_ISGID stripping doesn't apply. The ->symlink() and ->link() inode operations trivially inherit the mode from the target and the ->rename() inode operation inherits the mode from the source inode. All other creation inode operations will get S_ISGID handling via vfs_prepare_mode() when called from their relevant vfs_*() helpers. In addition to this there are filesystems which allow the creation of filesystem objects through ioctl()s or - in the case of spufs - circumventing the vfs in other ways. If filesystem objects are created through ioctl()s the vfs doesn't know about it and can't apply regular permission checking including S_ISGID logic. Therfore, a filesystem relying on S_ISGID stripping in inode_init_owner() in their ioctl() callpath will be affected by moving this logic into the vfs. We audited those filesystems: * btrfs allows the creation of filesystem objects through various ioctls(). Snapshot creation literally takes a snapshot and so the mode is fully preserved and S_ISGID stripping doesn't apply. Creating a new subvolum relies on inode_init_owner() in btrfs_new_subvol_inode() but only creates directories and doesn't raise S_ISGID. * ocfs2 has a peculiar implementation of reflinks. In contrast to e.g. xfs and btrfs FICLONE/FICLONERANGE ioctl() that is only concerned with the actual extents ocfs2 uses a separate ioctl() that also creates the target file. Iow, ocfs2 circumvents the vfs entirely here and did indeed rely on inode_init_owner() to strip the S_ISGID bit. This is the only place where a filesystem needs to call mode_strip_sgid() directly but this is self-inflicted pain. * spufs doesn't go through the vfs at all and doesn't use ioctl()s either. Instead it has a dedicated system call spufs_create() which allows the creation of filesystem objects. But spufs only creates directories and doesn't allo S_SIGID bits, i.e. it specifically only allows 0777 bits. * bpf uses vfs_mkobj() but also doesn't allow S_ISGID bits to be created. The patch will have an effect on ext2 when the EXT2_MOUNT_GRPID mount option is used, on ext4 when the EXT4_MOUNT_GRPID mount option is used, and on xfs when the XFS_FEAT_GRPID mount option is used. When any of these filesystems are mounted with their respective GRPID option then newly created files inherit the parent directories group unconditionally. In these cases non of the filesystems call inode_init_owner() and thus did never strip the S_ISGID bit for newly created files. Moving this logic into the VFS means that they now get the S_ISGID bit stripped. This is a user visible change. If this leads to regressions we will either need to figure out a better way or we need to revert. However, given the various setgid bugs that we found just in the last two years this is a regression risk we should take. Associated with this change is a new set of fstests to enforce the semantics for all new filesystems. Link: https://lore.kernel.org/ceph-devel/20220427092201.wvsdjbnc7b4dttaw@wittgenstein [1] Link: e014f37db1a2 ("xfs: use setattr_copy to set vfs inode attributes") [2] Link: 01ea173e103e ("xfs: fix up non-directory creation in SGID directories") [3] Link: fd84bfdddd16 ("ceph: fix up non-directory creation in SGID directories") [4] Link: https://lore.kernel.org/r/1657779088-2242-3-git-send-email-xuyang2018.jy@fujitsu.com Suggested-by: Dave Chinner Suggested-by: Christian Brauner (Microsoft) Reviewed-by: Darrick J. Wong Reviewed-and-Tested-by: Jeff Layton Signed-off-by: Yang Xu [: rewrote commit message] Signed-off-by: Christian Brauner (Microsoft) Signed-off-by: Amir Goldstein --- fs/inode.c | 2 -- fs/namei.c | 80 ++++++++++++++++++++++++++++++++++++++++-------- fs/ocfs2/namei.c | 1 + 3 files changed, 68 insertions(+), 15 deletions(-) diff --git a/fs/inode.c b/fs/inode.c index 23d03abcb0ff..52f834b6a3ad 100644 --- a/fs/inode.c +++ b/fs/inode.c @@ -2147,8 +2147,6 @@ void inode_init_owner(struct inode *inode, const struct inode *dir, /* Directories are special, and always inherit S_ISGID */ if (S_ISDIR(mode)) mode |= S_ISGID; - else - mode = mode_strip_sgid(dir, mode); } else inode->i_gid = current_fsgid(); inode->i_mode = mode; diff --git a/fs/namei.c b/fs/namei.c index 4159c140fa47..3d98db9802a7 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -2798,6 +2798,63 @@ void unlock_rename(struct dentry *p1, struct dentry *p2) } EXPORT_SYMBOL(unlock_rename); +/** + * mode_strip_umask - handle vfs umask stripping + * @dir: parent directory of the new inode + * @mode: mode of the new inode to be created in @dir + * + * Umask stripping depends on whether or not the filesystem supports POSIX + * ACLs. If the filesystem doesn't support it umask stripping is done directly + * in here. If the filesystem does support POSIX ACLs umask stripping is + * deferred until the filesystem calls posix_acl_create(). + * + * Returns: mode + */ +static inline umode_t mode_strip_umask(const struct inode *dir, umode_t mode) +{ + if (!IS_POSIXACL(dir)) + mode &= ~current_umask(); + return mode; +} + +/** + * vfs_prepare_mode - prepare the mode to be used for a new inode + * @dir: parent directory of the new inode + * @mode: mode of the new inode + * @mask_perms: allowed permission by the vfs + * @type: type of file to be created + * + * This helper consolidates and enforces vfs restrictions on the @mode of a new + * object to be created. + * + * Umask stripping depends on whether the filesystem supports POSIX ACLs (see + * the kernel documentation for mode_strip_umask()). Moving umask stripping + * after setgid stripping allows the same ordering for both non-POSIX ACL and + * POSIX ACL supporting filesystems. + * + * Note that it's currently valid for @type to be 0 if a directory is created. + * Filesystems raise that flag individually and we need to check whether each + * filesystem can deal with receiving S_IFDIR from the vfs before we enforce a + * non-zero type. + * + * Returns: mode to be passed to the filesystem + */ +static inline umode_t vfs_prepare_mode(const struct inode *dir, umode_t mode, + umode_t mask_perms, umode_t type) +{ + mode = mode_strip_sgid(dir, mode); + mode = mode_strip_umask(dir, mode); + + /* + * Apply the vfs mandated allowed permission mask and set the type of + * file to be created before we call into the filesystem. + */ + mode &= (mask_perms & ~S_IFMT); + mode |= (type & S_IFMT); + + return mode; +} + int vfs_create(struct inode *dir, struct dentry *dentry, umode_t mode, bool want_excl) { @@ -2807,8 +2864,8 @@ int vfs_create(struct inode *dir, struct dentry *dentry, umode_t mode, if (!dir->i_op->create) return -EACCES; /* shouldn't it be ENOSYS? */ - mode &= S_IALLUGO; - mode |= S_IFREG; + + mode = vfs_prepare_mode(dir, mode, S_IALLUGO, S_IFREG); error = security_inode_create(dir, dentry, mode); if (error) return error; @@ -3072,8 +3129,7 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file, if (open_flag & O_CREAT) { if (open_flag & O_EXCL) open_flag &= ~O_TRUNC; - if (!IS_POSIXACL(dir->d_inode)) - mode &= ~current_umask(); + mode = vfs_prepare_mode(dir->d_inode, mode, mode, mode); if (likely(got_write)) create_error = may_o_create(&nd->path, dentry, mode); else @@ -3286,8 +3342,7 @@ struct dentry *vfs_tmpfile(struct dentry *dentry, umode_t mode, int open_flag) child = d_alloc(dentry, &slash_name); if (unlikely(!child)) goto out_err; - if (!IS_POSIXACL(dir)) - mode &= ~current_umask(); + mode = vfs_prepare_mode(dir, mode, mode, mode); error = dir->i_op->tmpfile(dir, child, mode); if (error) goto out_err; @@ -3548,6 +3603,7 @@ int vfs_mknod(struct inode *dir, struct dentry *dentry, umode_t mode, dev_t dev) if (!dir->i_op->mknod) return -EPERM; + mode = vfs_prepare_mode(dir, mode, mode, mode); error = devcgroup_inode_mknod(mode, dev); if (error) return error; @@ -3596,9 +3652,8 @@ static long do_mknodat(int dfd, const char __user *filename, umode_t mode, if (IS_ERR(dentry)) return PTR_ERR(dentry); - if (!IS_POSIXACL(path.dentry->d_inode)) - mode &= ~current_umask(); - error = security_path_mknod(&path, dentry, mode, dev); + error = security_path_mknod(&path, dentry, + mode_strip_umask(path.dentry->d_inode, mode), dev); if (error) goto out; switch (mode & S_IFMT) { @@ -3646,7 +3701,7 @@ int vfs_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode) if (!dir->i_op->mkdir) return -EPERM; - mode &= (S_IRWXUGO|S_ISVTX); + mode = vfs_prepare_mode(dir, mode, S_IRWXUGO | S_ISVTX, 0); error = security_inode_mkdir(dir, dentry, mode); if (error) return error; @@ -3673,9 +3728,8 @@ static long do_mkdirat(int dfd, const char __user *pathname, umode_t mode) if (IS_ERR(dentry)) return PTR_ERR(dentry); - if (!IS_POSIXACL(path.dentry->d_inode)) - mode &= ~current_umask(); - error = security_path_mkdir(&path, dentry, mode); + error = security_path_mkdir(&path, dentry, + mode_strip_umask(path.dentry->d_inode, mode)); if (!error) error = vfs_mkdir(path.dentry->d_inode, dentry, mode); done_path_create(&path, dentry); diff --git a/fs/ocfs2/namei.c b/fs/ocfs2/namei.c index 856474b0a1ae..df1f6b7aa797 100644 --- a/fs/ocfs2/namei.c +++ b/fs/ocfs2/namei.c @@ -198,6 +198,7 @@ static struct inode *ocfs2_get_init_inode(struct inode *dir, umode_t mode) * callers. */ if (S_ISDIR(mode)) set_nlink(inode, 2); + mode = mode_strip_sgid(dir, mode); inode_init_owner(inode, dir, mode); status = dquot_initialize(inode); if (status) From patchwork Sat Mar 18 10:15:24 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Amir Goldstein X-Patchwork-Id: 13179736 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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id B2F8FC761AF for ; Sat, 18 Mar 2023 10:16:11 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229943AbjCRKQJ (ORCPT ); Sat, 18 Mar 2023 06:16:09 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55208 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229948AbjCRKP6 (ORCPT ); Sat, 18 Mar 2023 06:15:58 -0400 Received: from mail-wm1-x332.google.com (mail-wm1-x332.google.com [IPv6:2a00:1450:4864:20::332]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C028F25979; Sat, 18 Mar 2023 03:15:53 -0700 (PDT) Received: by mail-wm1-x332.google.com with SMTP id ay8so4684204wmb.1; Sat, 18 Mar 2023 03:15:53 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1679134552; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=eatdB0z21nbZqAoMxbWVck3KnIwW4SpkowjlzDGzwpw=; b=cw/GL07s5GJ7LGkZwE5KnszWaWwlfGC5OQ4vWbU0UNdAN3QGMf9VZ1CPqfrJhXmjYh VXZSYAtSbtPxeUu7p8yLW4MHAZHwSyvn/QOvzAde7eO/+ajdYzDkPBXXlrfl1uy/tMeI FIkU5qrtHwtKXk1BAiZllGxAwIWD8oVNtqSh8BnMmQKzb/vqVht0orBBbfg8J/IlpKMn i35be5oEb/vXfi3Cvcrzp9kA/2qDj7eJeyCsxg3F18p/IJVqwHvjtCbQ2RddKcJVioPA CYFaqsmwWo8wgnQOMC07utcAWWvbMO8eIipAAhM1hANIwh05VS5H9ygV4lAGRa0Gxsz0 xFmw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1679134552; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=eatdB0z21nbZqAoMxbWVck3KnIwW4SpkowjlzDGzwpw=; b=msUhMErN4kZv5u2vfQ9maPhofhjFM/XchCfORjMtIoUyKr3nx0pfCCljUc802+Bu6N ihMa2wJjCj+MVqTFr+/aybk/Bb7d91m27C/O38JCBjOTbaBiMBAKu5qm9VpTDOiw9KMh cF9FPoF1nVo98Bc7L2E5fsbrnwdkKFRBiovVxIbu+/YQ9RQOkCYzK6hZ1z+D8e6u6M7U ra2poxlpCzS1TW283PJBYHxbQ4t7Cc+b84SpCgOeH9E/Etuc48mPPHfXOyvfSfvxt+QD issDBhtL+SwbtLmZgeEHROABSbB/CUS3NGuDU/hGmfNv3jR6DixAuOsa5Ucb+VoGxgGk VDlA== X-Gm-Message-State: AO0yUKXQO1BTex2z3sn9SWqgw5ySnQgLQihbk7eKETtwFEi/pU8AS3Bu amSxv4Om7Z6p8RK5p5sFURE= X-Google-Smtp-Source: AK7set+nDV4tJfhxDK+u/YypvydP13hNKDyvgfWE7uzwncRytdDK4nvoGnNow6eSvdvT3ckEk2oDFA== X-Received: by 2002:a05:600c:3b14:b0:3ed:29f8:3ff2 with SMTP id m20-20020a05600c3b1400b003ed29f83ff2mr4511089wms.6.1679134552376; Sat, 18 Mar 2023 03:15:52 -0700 (PDT) Received: from amir-ThinkPad-T480.lan ([5.29.249.86]) by smtp.gmail.com with ESMTPSA id v26-20020a05600c215a00b003eafc47eb09sm4333965wml.43.2023.03.18.03.15.51 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 18 Mar 2023 03:15:52 -0700 (PDT) From: Amir Goldstein To: Greg Kroah-Hartman Cc: Sasha Levin , "Darrick J . Wong" , Leah Rumancik , Chandan Babu R , Christian Brauner , linux-fsdevel@vger.kernel.org, linux-xfs@vger.kernel.org, stable@vger.kernel.org Subject: [PATCH 5.10 10/15] attr: add in_group_or_capable() Date: Sat, 18 Mar 2023 12:15:24 +0200 Message-Id: <20230318101529.1361673-11-amir73il@gmail.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20230318101529.1361673-1-amir73il@gmail.com> References: <20230318101529.1361673-1-amir73il@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org commit 11c2a8700cdcabf9b639b7204a1e38e2a0b6798e upstream. [backported to 5.10.y, prior to idmapped mounts] In setattr_{copy,prepare}() we need to perform the same permission checks to determine whether we need to drop the setgid bit or not. Instead of open-coding it twice add a simple helper the encapsulates the logic. We will reuse this helpers to make dropping the setgid bit during write operations more consistent in a follow up patch. Reviewed-by: Amir Goldstein Signed-off-by: Christian Brauner (Microsoft) Signed-off-by: Amir Goldstein --- fs/attr.c | 11 +++++------ fs/inode.c | 25 +++++++++++++++++++++---- fs/internal.h | 1 + 3 files changed, 27 insertions(+), 10 deletions(-) diff --git a/fs/attr.c b/fs/attr.c index 848ffe6e3c24..300ba5153868 100644 --- a/fs/attr.c +++ b/fs/attr.c @@ -18,6 +18,8 @@ #include #include +#include "internal.h" + static bool chown_ok(const struct inode *inode, kuid_t uid) { if (uid_eq(current_fsuid(), inode->i_uid) && @@ -90,9 +92,8 @@ int setattr_prepare(struct dentry *dentry, struct iattr *attr) if (!inode_owner_or_capable(inode)) return -EPERM; /* Also check the setgid bit! */ - if (!in_group_p((ia_valid & ATTR_GID) ? attr->ia_gid : - inode->i_gid) && - !capable_wrt_inode_uidgid(inode, CAP_FSETID)) + if (!in_group_or_capable(inode, (ia_valid & ATTR_GID) ? + attr->ia_gid : inode->i_gid)) attr->ia_mode &= ~S_ISGID; } @@ -193,9 +194,7 @@ void setattr_copy(struct inode *inode, const struct iattr *attr) inode->i_ctime = attr->ia_ctime; if (ia_valid & ATTR_MODE) { umode_t mode = attr->ia_mode; - - if (!in_group_p(inode->i_gid) && - !capable_wrt_inode_uidgid(inode, CAP_FSETID)) + if (!in_group_or_capable(inode, inode->i_gid)) mode &= ~S_ISGID; inode->i_mode = mode; } diff --git a/fs/inode.c b/fs/inode.c index 52f834b6a3ad..63f86aeda7fd 100644 --- a/fs/inode.c +++ b/fs/inode.c @@ -2379,6 +2379,26 @@ int vfs_ioc_fssetxattr_check(struct inode *inode, const struct fsxattr *old_fa, } EXPORT_SYMBOL(vfs_ioc_fssetxattr_check); +/** + * in_group_or_capable - check whether caller is CAP_FSETID privileged + * @inode: inode to check + * @gid: the new/current gid of @inode + * + * Check wether @gid is in the caller's group list or if the caller is + * privileged with CAP_FSETID over @inode. This can be used to determine + * whether the setgid bit can be kept or must be dropped. + * + * Return: true if the caller is sufficiently privileged, false if not. + */ +bool in_group_or_capable(const struct inode *inode, kgid_t gid) +{ + if (in_group_p(gid)) + return true; + if (capable_wrt_inode_uidgid(inode, CAP_FSETID)) + return true; + return false; +} + /** * mode_strip_sgid - handle the sgid bit for non-directories * @dir: parent directory inode @@ -2398,11 +2418,8 @@ umode_t mode_strip_sgid(const struct inode *dir, umode_t mode) return mode; if (S_ISDIR(mode) || !dir || !(dir->i_mode & S_ISGID)) return mode; - if (in_group_p(dir->i_gid)) - return mode; - if (capable_wrt_inode_uidgid(dir, CAP_FSETID)) + if (in_group_or_capable(dir, dir->i_gid)) return mode; - return mode & ~S_ISGID; } EXPORT_SYMBOL(mode_strip_sgid); diff --git a/fs/internal.h b/fs/internal.h index 06d313b9beec..0fe920d9f393 100644 --- a/fs/internal.h +++ b/fs/internal.h @@ -149,6 +149,7 @@ extern int vfs_open(const struct path *, struct file *); extern long prune_icache_sb(struct super_block *sb, struct shrink_control *sc); extern void inode_add_lru(struct inode *inode); extern int dentry_needs_remove_privs(struct dentry *dentry); +bool in_group_or_capable(const struct inode *inode, kgid_t gid); /* * fs-writeback.c From patchwork Sat Mar 18 10:15:25 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Amir Goldstein X-Patchwork-Id: 13179738 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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id E9909C77B61 for ; Sat, 18 Mar 2023 10:16:12 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229848AbjCRKQL (ORCPT ); Sat, 18 Mar 2023 06:16:11 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54998 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229959AbjCRKP6 (ORCPT ); Sat, 18 Mar 2023 06:15:58 -0400 Received: from mail-wm1-x332.google.com (mail-wm1-x332.google.com [IPv6:2a00:1450:4864:20::332]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 3DB96279A5; Sat, 18 Mar 2023 03:15:54 -0700 (PDT) Received: by mail-wm1-x332.google.com with SMTP id fm20-20020a05600c0c1400b003ead37e6588so6405116wmb.5; Sat, 18 Mar 2023 03:15:54 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1679134554; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=hNVncBi+eLx3DXaqCOJRdjKPPS5ZKYBqno7LFhDYHyo=; b=JH9SEF/ey2pweaLNV7mCepNRPDqcG38XZ6lsqEgz6GiLnpJ+ZyPdCh9NjDLp3+kVO7 UIGir8NsTMaYk6XYDlhCahKfunhurLSKM1clpIgWA/E7UMYw8uIXMhN2Az2jV8ZZqnFq uFv3pevOFm15bJnAcdDfyn3PrIm4+9NBzXLAJmn+uozBRdJcvr2EQlIMQtWRoQGYi80H JSl/72Ab858npJxZEH3MPQQi7ZX8KirB0dvHwF35n26/Iro1/ygK0e4IFS/0okjKABOO zj5Kd1YizHV4n82tqmbTFRdd8kT5dudSkZAwDDA8ZfRLJRs0x+lvSzvD9FpSQ7SGkZ6o C5Qg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1679134554; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=hNVncBi+eLx3DXaqCOJRdjKPPS5ZKYBqno7LFhDYHyo=; b=5EM9J4XulQA4SJMeAB+mmOzNHCZb0uInEpz28aABRvELHA4Z4uuJYMvQEOfvV04hbs gQpaPRGXSLyQaC9wNEhS6JHgjXbd7tWFsbucxZ7PwJse0HX5T9vYcctE9x+6Ivqt+TFN JWEbq0LjOag1Q0nywu4QpzY2rBMDGy8zfX3nvzW3+a2AFjgOMh6TdvIhAzTiC6R3Csd7 yyZy2l/TTD93rQYA7sdBGo2yrB4es9me9zmMBv8o94hpyY+33WTEmQ4xQ5c9OKk+WNh+ pLdYoAnzHoCBXKAVpi8dat7/iNUTKL05zOS3s000Y6dEGtE9c90s/8Jg2XZoMaSYWnuU q+EQ== X-Gm-Message-State: AO0yUKWzq+DgFxOUXjbwCYAeFFP0rQ+6wyWpGBB55APjAbAH+zHBdb4V wLQWJIieVDa0eDYZ5D3/W1AZo74YObA= X-Google-Smtp-Source: AK7set9ZwL7TEXVhKEgYtnfgytaWXHs6ZSpYij5gPF80VJTPA0CYOMAlwYJvX7rM0vcDhNFrh9BI0g== X-Received: by 2002:a05:600c:4f8e:b0:3e2:662:ade6 with SMTP id n14-20020a05600c4f8e00b003e20662ade6mr26346649wmq.26.1679134553874; Sat, 18 Mar 2023 03:15:53 -0700 (PDT) Received: from amir-ThinkPad-T480.lan ([5.29.249.86]) by smtp.gmail.com with ESMTPSA id v26-20020a05600c215a00b003eafc47eb09sm4333965wml.43.2023.03.18.03.15.52 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 18 Mar 2023 03:15:53 -0700 (PDT) From: Amir Goldstein To: Greg Kroah-Hartman Cc: Sasha Levin , "Darrick J . Wong" , Leah Rumancik , Chandan Babu R , Christian Brauner , linux-fsdevel@vger.kernel.org, linux-xfs@vger.kernel.org, stable@vger.kernel.org Subject: [PATCH 5.10 11/15] fs: move should_remove_suid() Date: Sat, 18 Mar 2023 12:15:25 +0200 Message-Id: <20230318101529.1361673-12-amir73il@gmail.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20230318101529.1361673-1-amir73il@gmail.com> References: <20230318101529.1361673-1-amir73il@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org commit e243e3f94c804ecca9a8241b5babe28f35258ef4 upstream. Move the helper from inode.c to attr.c. This keeps the the core of the set{g,u}id stripping logic in one place when we add follow-up changes. It is the better place anyway, since should_remove_suid() returns ATTR_KILL_S{G,U}ID flags. Reviewed-by: Amir Goldstein Signed-off-by: Christian Brauner (Microsoft) Signed-off-by: Amir Goldstein --- fs/attr.c | 29 +++++++++++++++++++++++++++++ fs/inode.c | 29 ----------------------------- 2 files changed, 29 insertions(+), 29 deletions(-) diff --git a/fs/attr.c b/fs/attr.c index 300ba5153868..666489157978 100644 --- a/fs/attr.c +++ b/fs/attr.c @@ -20,6 +20,35 @@ #include "internal.h" +/* + * The logic we want is + * + * if suid or (sgid and xgrp) + * remove privs + */ +int should_remove_suid(struct dentry *dentry) +{ + umode_t mode = d_inode(dentry)->i_mode; + int kill = 0; + + /* suid always must be killed */ + if (unlikely(mode & S_ISUID)) + kill = ATTR_KILL_SUID; + + /* + * sgid without any exec bits is just a mandatory locking mark; leave + * it alone. If some exec bits are set, it's a real sgid; kill it. + */ + if (unlikely((mode & S_ISGID) && (mode & S_IXGRP))) + kill |= ATTR_KILL_SGID; + + if (unlikely(kill && !capable(CAP_FSETID) && S_ISREG(mode))) + return kill; + + return 0; +} +EXPORT_SYMBOL(should_remove_suid); + static bool chown_ok(const struct inode *inode, kuid_t uid) { if (uid_eq(current_fsuid(), inode->i_uid) && diff --git a/fs/inode.c b/fs/inode.c index 63f86aeda7fd..f52dd6feea98 100644 --- a/fs/inode.c +++ b/fs/inode.c @@ -1854,35 +1854,6 @@ void touch_atime(const struct path *path) } EXPORT_SYMBOL(touch_atime); -/* - * The logic we want is - * - * if suid or (sgid and xgrp) - * remove privs - */ -int should_remove_suid(struct dentry *dentry) -{ - umode_t mode = d_inode(dentry)->i_mode; - int kill = 0; - - /* suid always must be killed */ - if (unlikely(mode & S_ISUID)) - kill = ATTR_KILL_SUID; - - /* - * sgid without any exec bits is just a mandatory locking mark; leave - * it alone. If some exec bits are set, it's a real sgid; kill it. - */ - if (unlikely((mode & S_ISGID) && (mode & S_IXGRP))) - kill |= ATTR_KILL_SGID; - - if (unlikely(kill && !capable(CAP_FSETID) && S_ISREG(mode))) - return kill; - - return 0; -} -EXPORT_SYMBOL(should_remove_suid); - /* * Return mask of changes for notify_change() that need to be done as a * response to write or truncate. Return 0 if nothing has to be changed. From patchwork Sat Mar 18 10:15:26 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Amir Goldstein X-Patchwork-Id: 13179739 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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id A027AC76195 for ; Sat, 18 Mar 2023 10:16:14 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229991AbjCRKQM (ORCPT ); Sat, 18 Mar 2023 06:16:12 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55242 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229963AbjCRKQA (ORCPT ); Sat, 18 Mar 2023 06:16:00 -0400 Received: from mail-wm1-x331.google.com (mail-wm1-x331.google.com [IPv6:2a00:1450:4864:20::331]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D40F828233; Sat, 18 Mar 2023 03:15:55 -0700 (PDT) Received: by mail-wm1-x331.google.com with SMTP id p13-20020a05600c358d00b003ed346d4522so4726867wmq.2; Sat, 18 Mar 2023 03:15:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1679134555; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=Zs1zoKvdKc411P0VlWS94lq85UVPmglODGE22wnBLL0=; b=KtnFG4veDCHg3Voz4u1iXxnj+pdvMXjIOaIy8bBbOw8vrHe0trW/RziV4WsqcO+4f2 OTY8AQq3unLJ5JLieWF4N+kM9VfsYUPGHrX+QhUZzWak7QRVjTct+PiJwynnLdrfvXaq fhwYiabRq0yLH15yi9yieJUJTehEPxPYj3q2vSOBS5Q13jTG8q25YzOHuWevlLvDrEZk x7xe6o0mf/ijTn4Z0clckC7qvsoZoIaAhtpBpIAbeTpsoaRNg5XGaSo5HvL4+L63oAFw 1O7PxBiDk+NbBhsuvpVS3/4fDfuir5y4JwrqK/XRVdSnft7fNEP7VG8qyAulG7THj4Lt vIcw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1679134555; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=Zs1zoKvdKc411P0VlWS94lq85UVPmglODGE22wnBLL0=; b=ICI/scQcTxCQSk1PlDuPAnL9zbjtAtvs96orkR0chDmNfFSoIEw5hj0V4eGB647qvs ocIsoVcMuF7OuYBQX8jByhdJt1vW5J9/RZC3q+qKQnPsz0QeKPOgCe4ybSrKP1If/KEH rBT57uF2NOU12KKaQ46P3n+NzPo2b4czS4+S5BJ/aYrrrs/Ki6GJzOl8ZcrQ7SMcKPXC ZY1zs2VS6yHnz7lxoEUZDbH5o986kWVayxDVB6NQEgySQcy+nq9a0EBUK9XM892h1SbR LhgPRusz2c1aawNvfd8hq2ofbNHeB+nUHcC2Zf8IDGFfO1suu9U1AJvTBS0hegqbhxEy fALg== X-Gm-Message-State: AO0yUKV1mTEZ3GMa+PjPzaNKBn+IjjlELiCNpCHhxoDLmvqFiBhPfBke 9gAGs/8MXxb2vNYp3YgRIZc= X-Google-Smtp-Source: AK7set8dNJLI8k5rlP0DqIkQFV5x8exo3w8ajCaQVWtYzDUNTDw9Cc5hS7ZuuSze3NukIP4Q0OwDwg== X-Received: by 2002:a05:600c:548b:b0:3e2:6ec:61ea with SMTP id iv11-20020a05600c548b00b003e206ec61eamr27912401wmb.28.1679134555474; Sat, 18 Mar 2023 03:15:55 -0700 (PDT) Received: from amir-ThinkPad-T480.lan ([5.29.249.86]) by smtp.gmail.com with ESMTPSA id v26-20020a05600c215a00b003eafc47eb09sm4333965wml.43.2023.03.18.03.15.54 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 18 Mar 2023 03:15:55 -0700 (PDT) From: Amir Goldstein To: Greg Kroah-Hartman Cc: Sasha Levin , "Darrick J . Wong" , Leah Rumancik , Chandan Babu R , Christian Brauner , linux-fsdevel@vger.kernel.org, linux-xfs@vger.kernel.org, stable@vger.kernel.org Subject: [PATCH 5.10 12/15] attr: add setattr_should_drop_sgid() Date: Sat, 18 Mar 2023 12:15:26 +0200 Message-Id: <20230318101529.1361673-13-amir73il@gmail.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20230318101529.1361673-1-amir73il@gmail.com> References: <20230318101529.1361673-1-amir73il@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org commit 72ae017c5451860443a16fb2a8c243bff3e396b8 upstream. [backported to 5.10.y, prior to idmapped mounts] The current setgid stripping logic during write and ownership change operations is inconsistent and strewn over multiple places. In order to consolidate it and make more consistent we'll add a new helper setattr_should_drop_sgid(). The function retains the old behavior where we remove the S_ISGID bit unconditionally when S_IXGRP is set but also when it isn't set and the caller is neither in the group of the inode nor privileged over the inode. We will use this helper both in write operation permission removal such as file_remove_privs() as well as in ownership change operations. Reviewed-by: Amir Goldstein Signed-off-by: Christian Brauner (Microsoft) Signed-off-by: Amir Goldstein --- fs/attr.c | 25 +++++++++++++++++++++++++ fs/internal.h | 5 +++++ 2 files changed, 30 insertions(+) diff --git a/fs/attr.c b/fs/attr.c index 666489157978..c8049ae34a2e 100644 --- a/fs/attr.c +++ b/fs/attr.c @@ -20,6 +20,31 @@ #include "internal.h" +/** + * setattr_should_drop_sgid - determine whether the setgid bit needs to be + * removed + * @inode: inode to check + * + * This function determines whether the setgid bit needs to be removed. + * We retain backwards compatibility and require setgid bit to be removed + * unconditionally if S_IXGRP is set. Otherwise we have the exact same + * requirements as setattr_prepare() and setattr_copy(). + * + * Return: ATTR_KILL_SGID if setgid bit needs to be removed, 0 otherwise. + */ +int setattr_should_drop_sgid(const struct inode *inode) +{ + umode_t mode = inode->i_mode; + + if (!(mode & S_ISGID)) + return 0; + if (mode & S_IXGRP) + return ATTR_KILL_SGID; + if (!in_group_or_capable(inode, inode->i_gid)) + return ATTR_KILL_SGID; + return 0; +} + /* * The logic we want is * diff --git a/fs/internal.h b/fs/internal.h index 0fe920d9f393..d5d9fcdae10c 100644 --- a/fs/internal.h +++ b/fs/internal.h @@ -197,3 +197,8 @@ int sb_init_dio_done_wq(struct super_block *sb); */ int do_statx(int dfd, const char __user *filename, unsigned flags, unsigned int mask, struct statx __user *buffer); + +/* + * fs/attr.c + */ +int setattr_should_drop_sgid(const struct inode *inode); From patchwork Sat Mar 18 10:15:27 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Amir Goldstein X-Patchwork-Id: 13179741 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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3CF8FC7618E for ; Sat, 18 Mar 2023 10:16:15 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230003AbjCRKQN (ORCPT ); Sat, 18 Mar 2023 06:16:13 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54914 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229970AbjCRKQA (ORCPT ); Sat, 18 Mar 2023 06:16:00 -0400 Received: from mail-wm1-x332.google.com (mail-wm1-x332.google.com [IPv6:2a00:1450:4864:20::332]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 78F3D2E0C0; Sat, 18 Mar 2023 03:15:57 -0700 (PDT) Received: by mail-wm1-x332.google.com with SMTP id ay8so4684270wmb.1; Sat, 18 Mar 2023 03:15:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1679134557; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=wEnBZ7cY3XwVYz9pj+LbW9TyrRM8DzdvnhMYIo+uxZw=; b=fbGHo159do+XtZssQcCnfMMlG5/rzPzEkNCdW6iT6l9LLe5A6nfyEwgvfDUiF3U6Ux xIP0I4QFi4ZBN0U9pnlz2m8TW/blBhwhD1bV5HwdyrEC2v8QNit3UvNRLOZeQa7OqhvD /UxwMYZV+caxLr74dslORG99VzBd+QRQK7BmcxBGhQ6cC2/aFInvmzcoiYiuoI8ABm0i AdmWdCMCXYxNAtK65qt5TMIu1Rk6ydD+Wa/t1w/MNoS28S33X1wlq5fV8HFPFaKUffr6 htVpOOl6FwRyuqXF29ghOgi+SYJ4wERM2tzqbqFc3gyQqyA+gkc1W7kgmASuayiXrnA9 XO+g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1679134557; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=wEnBZ7cY3XwVYz9pj+LbW9TyrRM8DzdvnhMYIo+uxZw=; b=T79o9+EdWV2quG7D8QRLHo99ZAWcRTMR8/ujqm926C3DdJg0en2r9RPFxONfQotjbk yVwuWTShYoFhcJM3/3UU3VEn7cmk9n327cXBbhEP0ji7hQcCE2vza+j5sVRRJ31qLcLe jb7VVTvdR532SdHmwDVBnmg3F67tMrdHbNZjZGQ9p04/aresn86cfOazZB0U1hNjbDum +7lnAmM2EfktBSCcEKww/3HGQkv5SzdGqn+X7dJqh7ORtLq/YAT6/NalqZSPXBqTp6VT azTE/KzMFVFFh3bRZeXuGVw2ktdSuz8o3BVZFQ8am+orxJpsAsOLJingjgJ++yLt0nII 4mWw== X-Gm-Message-State: AO0yUKVrqDQXcPYCOcCny302hDgpbu2kt2CPKNkaVbVFhDrKIqYfbGoV qbj7EtfqD/XKFAB5vq09E/o= X-Google-Smtp-Source: AK7set8QwCe2Kwb9o0b9bDcb4bwXcR8Dt32x2lXw+rRFw62oaiNq5RwN0B8ajYuveLhOOcYhn99FKw== X-Received: by 2002:a7b:c445:0:b0:3ed:abb9:7501 with SMTP id l5-20020a7bc445000000b003edabb97501mr1235964wmi.23.1679134556909; Sat, 18 Mar 2023 03:15:56 -0700 (PDT) Received: from amir-ThinkPad-T480.lan ([5.29.249.86]) by smtp.gmail.com with ESMTPSA id v26-20020a05600c215a00b003eafc47eb09sm4333965wml.43.2023.03.18.03.15.55 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 18 Mar 2023 03:15:56 -0700 (PDT) From: Amir Goldstein To: Greg Kroah-Hartman Cc: Sasha Levin , "Darrick J . Wong" , Leah Rumancik , Chandan Babu R , Christian Brauner , linux-fsdevel@vger.kernel.org, linux-xfs@vger.kernel.org, stable@vger.kernel.org Subject: [PATCH 5.10 13/15] attr: use consistent sgid stripping checks Date: Sat, 18 Mar 2023 12:15:27 +0200 Message-Id: <20230318101529.1361673-14-amir73il@gmail.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20230318101529.1361673-1-amir73il@gmail.com> References: <20230318101529.1361673-1-amir73il@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org commit ed5a7047d2011cb6b2bf84ceb6680124cc6a7d95 upstream. [backported to 5.10.y, prior to idmapped mounts] Currently setgid stripping in file_remove_privs()'s should_remove_suid() helper is inconsistent with other parts of the vfs. Specifically, it only raises ATTR_KILL_SGID if the inode is S_ISGID and S_IXGRP but not if the inode isn't in the caller's groups and the caller isn't privileged over the inode although we require this already in setattr_prepare() and setattr_copy() and so all filesystem implement this requirement implicitly because they have to use setattr_{prepare,copy}() anyway. But the inconsistency shows up in setgid stripping bugs for overlayfs in xfstests (e.g., generic/673, generic/683, generic/685, generic/686, generic/687). For example, we test whether suid and setgid stripping works correctly when performing various write-like operations as an unprivileged user (fallocate, reflink, write, etc.): echo "Test 1 - qa_user, non-exec file $verb" setup_testfile chmod a+rws $junk_file commit_and_check "$qa_user" "$verb" 64k 64k The test basically creates a file with 6666 permissions. While the file has the S_ISUID and S_ISGID bits set it does not have the S_IXGRP set. On a regular filesystem like xfs what will happen is: sys_fallocate() -> vfs_fallocate() -> xfs_file_fallocate() -> file_modified() -> __file_remove_privs() -> dentry_needs_remove_privs() -> should_remove_suid() -> __remove_privs() newattrs.ia_valid = ATTR_FORCE | kill; -> notify_change() -> setattr_copy() In should_remove_suid() we can see that ATTR_KILL_SUID is raised unconditionally because the file in the test has S_ISUID set. But we also see that ATTR_KILL_SGID won't be set because while the file is S_ISGID it is not S_IXGRP (see above) which is a condition for ATTR_KILL_SGID being raised. So by the time we call notify_change() we have attr->ia_valid set to ATTR_KILL_SUID | ATTR_FORCE. Now notify_change() sees that ATTR_KILL_SUID is set and does: ia_valid = attr->ia_valid |= ATTR_MODE attr->ia_mode = (inode->i_mode & ~S_ISUID); which means that when we call setattr_copy() later we will definitely update inode->i_mode. Note that attr->ia_mode still contains S_ISGID. Now we call into the filesystem's ->setattr() inode operation which will end up calling setattr_copy(). Since ATTR_MODE is set we will hit: if (ia_valid & ATTR_MODE) { umode_t mode = attr->ia_mode; vfsgid_t vfsgid = i_gid_into_vfsgid(mnt_userns, inode); if (!vfsgid_in_group_p(vfsgid) && !capable_wrt_inode_uidgid(mnt_userns, inode, CAP_FSETID)) mode &= ~S_ISGID; inode->i_mode = mode; } and since the caller in the test is neither capable nor in the group of the inode the S_ISGID bit is stripped. But assume the file isn't suid then ATTR_KILL_SUID won't be raised which has the consequence that neither the setgid nor the suid bits are stripped even though it should be stripped because the inode isn't in the caller's groups and the caller isn't privileged over the inode. If overlayfs is in the mix things become a bit more complicated and the bug shows up more clearly. When e.g., ovl_setattr() is hit from ovl_fallocate()'s call to file_remove_privs() then ATTR_KILL_SUID and ATTR_KILL_SGID might be raised but because the check in notify_change() is questioning the ATTR_KILL_SGID flag again by requiring S_IXGRP for it to be stripped the S_ISGID bit isn't removed even though it should be stripped: sys_fallocate() -> vfs_fallocate() -> ovl_fallocate() -> file_remove_privs() -> dentry_needs_remove_privs() -> should_remove_suid() -> __remove_privs() newattrs.ia_valid = ATTR_FORCE | kill; -> notify_change() -> ovl_setattr() // TAKE ON MOUNTER'S CREDS -> ovl_do_notify_change() -> notify_change() // GIVE UP MOUNTER'S CREDS // TAKE ON MOUNTER'S CREDS -> vfs_fallocate() -> xfs_file_fallocate() -> file_modified() -> __file_remove_privs() -> dentry_needs_remove_privs() -> should_remove_suid() -> __remove_privs() newattrs.ia_valid = attr_force | kill; -> notify_change() The fix for all of this is to make file_remove_privs()'s should_remove_suid() helper to perform the same checks as we already require in setattr_prepare() and setattr_copy() and have notify_change() not pointlessly requiring S_IXGRP again. It doesn't make any sense in the first place because the caller must calculate the flags via should_remove_suid() anyway which would raise ATTR_KILL_SGID. While we're at it we move should_remove_suid() from inode.c to attr.c where it belongs with the rest of the iattr helpers. Especially since it returns ATTR_KILL_S{G,U}ID flags. We also rename it to setattr_should_drop_suidgid() to better reflect that it indicates both setuid and setgid bit removal and also that it returns attr flags. Running xfstests with this doesn't report any regressions. We should really try and use consistent checks. Reviewed-by: Amir Goldstein Signed-off-by: Christian Brauner (Microsoft) Signed-off-by: Amir Goldstein --- Documentation/trace/ftrace.rst | 2 +- fs/attr.c | 31 +++++++++++++++++-------------- fs/inode.c | 2 +- fs/ocfs2/file.c | 4 ++-- fs/open.c | 6 +++--- include/linux/fs.h | 2 +- 6 files changed, 25 insertions(+), 22 deletions(-) diff --git a/Documentation/trace/ftrace.rst b/Documentation/trace/ftrace.rst index 87cf5c010d5d..ed2e45f9b762 100644 --- a/Documentation/trace/ftrace.rst +++ b/Documentation/trace/ftrace.rst @@ -2923,7 +2923,7 @@ Produces:: bash-1994 [000] .... 4342.324898: ima_get_action <-process_measurement bash-1994 [000] .... 4342.324898: ima_match_policy <-ima_get_action bash-1994 [000] .... 4342.324899: do_truncate <-do_last - bash-1994 [000] .... 4342.324899: should_remove_suid <-do_truncate + bash-1994 [000] .... 4342.324899: setattr_should_drop_suidgid <-do_truncate bash-1994 [000] .... 4342.324899: notify_change <-do_truncate bash-1994 [000] .... 4342.324900: current_fs_time <-notify_change bash-1994 [000] .... 4342.324900: current_kernel_time <-current_fs_time diff --git a/fs/attr.c b/fs/attr.c index c8049ae34a2e..326a0db3296d 100644 --- a/fs/attr.c +++ b/fs/attr.c @@ -45,34 +45,37 @@ int setattr_should_drop_sgid(const struct inode *inode) return 0; } -/* - * The logic we want is +/** + * setattr_should_drop_suidgid - determine whether the set{g,u}id bit needs to + * be dropped + * @inode: inode to check + * + * This function determines whether the set{g,u}id bits need to be removed. + * If the setuid bit needs to be removed ATTR_KILL_SUID is returned. If the + * setgid bit needs to be removed ATTR_KILL_SGID is returned. If both + * set{g,u}id bits need to be removed the corresponding mask of both flags is + * returned. * - * if suid or (sgid and xgrp) - * remove privs + * Return: A mask of ATTR_KILL_S{G,U}ID indicating which - if any - setid bits + * to remove, 0 otherwise. */ -int should_remove_suid(struct dentry *dentry) +int setattr_should_drop_suidgid(struct inode *inode) { - umode_t mode = d_inode(dentry)->i_mode; + umode_t mode = inode->i_mode; int kill = 0; /* suid always must be killed */ if (unlikely(mode & S_ISUID)) kill = ATTR_KILL_SUID; - /* - * sgid without any exec bits is just a mandatory locking mark; leave - * it alone. If some exec bits are set, it's a real sgid; kill it. - */ - if (unlikely((mode & S_ISGID) && (mode & S_IXGRP))) - kill |= ATTR_KILL_SGID; + kill |= setattr_should_drop_sgid(inode); if (unlikely(kill && !capable(CAP_FSETID) && S_ISREG(mode))) return kill; return 0; } -EXPORT_SYMBOL(should_remove_suid); +EXPORT_SYMBOL(setattr_should_drop_suidgid); static bool chown_ok(const struct inode *inode, kuid_t uid) { @@ -350,7 +353,7 @@ int notify_change(struct dentry * dentry, struct iattr * attr, struct inode **de } } if (ia_valid & ATTR_KILL_SGID) { - if ((mode & (S_ISGID | S_IXGRP)) == (S_ISGID | S_IXGRP)) { + if (mode & S_ISGID) { if (!(ia_valid & ATTR_MODE)) { ia_valid = attr->ia_valid |= ATTR_MODE; attr->ia_mode = inode->i_mode; diff --git a/fs/inode.c b/fs/inode.c index f52dd6feea98..7ec90788d8be 100644 --- a/fs/inode.c +++ b/fs/inode.c @@ -1868,7 +1868,7 @@ int dentry_needs_remove_privs(struct dentry *dentry) if (IS_NOSEC(inode)) return 0; - mask = should_remove_suid(dentry); + mask = setattr_should_drop_suidgid(inode); ret = security_inode_need_killpriv(dentry); if (ret < 0) return ret; diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c index 1470b49adb2d..ca00cac5a12f 100644 --- a/fs/ocfs2/file.c +++ b/fs/ocfs2/file.c @@ -1994,7 +1994,7 @@ static int __ocfs2_change_file_space(struct file *file, struct inode *inode, } } - if (file && should_remove_suid(file->f_path.dentry)) { + if (file && setattr_should_drop_suidgid(file_inode(file))) { ret = __ocfs2_write_remove_suid(inode, di_bh); if (ret) { mlog_errno(ret); @@ -2282,7 +2282,7 @@ static int ocfs2_prepare_inode_for_write(struct file *file, * inode. There's also the dinode i_size state which * can be lost via setattr during extending writes (we * set inode->i_size at the end of a write. */ - if (should_remove_suid(dentry)) { + if (setattr_should_drop_suidgid(inode)) { if (meta_level == 0) { ocfs2_inode_unlock_for_extent_tree(inode, &di_bh, diff --git a/fs/open.c b/fs/open.c index b3fbb4300fc9..1ca4b236fdbe 100644 --- a/fs/open.c +++ b/fs/open.c @@ -665,10 +665,10 @@ int chown_common(const struct path *path, uid_t user, gid_t group) newattrs.ia_valid |= ATTR_GID; newattrs.ia_gid = gid; } - if (!S_ISDIR(inode->i_mode)) - newattrs.ia_valid |= - ATTR_KILL_SUID | ATTR_KILL_SGID | ATTR_KILL_PRIV; inode_lock(inode); + if (!S_ISDIR(inode->i_mode)) + newattrs.ia_valid |= ATTR_KILL_SUID | ATTR_KILL_PRIV | + setattr_should_drop_sgid(inode); error = security_path_chown(path, uid, gid); if (!error) error = notify_change(path->dentry, &newattrs, &delegated_inode); diff --git a/include/linux/fs.h b/include/linux/fs.h index 527791e4860b..57afa4fa5e7b 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -2960,7 +2960,7 @@ extern void __destroy_inode(struct inode *); extern struct inode *new_inode_pseudo(struct super_block *sb); extern struct inode *new_inode(struct super_block *sb); extern void free_inode_nonrcu(struct inode *inode); -extern int should_remove_suid(struct dentry *); +extern int setattr_should_drop_suidgid(struct inode *); extern int file_remove_privs(struct file *); extern void __insert_inode_hash(struct inode *, unsigned long hashval); From patchwork Sat Mar 18 10:15:28 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Amir Goldstein X-Patchwork-Id: 13179740 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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3373BC761AF for ; Sat, 18 Mar 2023 10:16:16 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229959AbjCRKQO (ORCPT ); Sat, 18 Mar 2023 06:16:14 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55330 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229940AbjCRKQB (ORCPT ); Sat, 18 Mar 2023 06:16:01 -0400 Received: from mail-wm1-x32e.google.com (mail-wm1-x32e.google.com [IPv6:2a00:1450:4864:20::32e]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 1C562392B6; Sat, 18 Mar 2023 03:15:59 -0700 (PDT) Received: by mail-wm1-x32e.google.com with SMTP id c8-20020a05600c0ac800b003ed2f97a63eso6419992wmr.3; Sat, 18 Mar 2023 03:15:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1679134558; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=fHjSWW/YZEGEGz44nnM9t/++2Yb+yS/AmGPiI8kwmpU=; b=l8Ua+bKX5fGvv0MYSPpTTcyP8HkXfmSEgfep+W5+19RyD7IsF+RZF/oHC6GtXz7g1L Nx82ja9W/XG0zztQgCynC5FBzxIZ7pAufeop9pg9QRvpYGv0wt97Qc6Tm46CbArbOKx+ McGbJR/Vd9ranrh5vssV/EC1jeCvwAYRrwOn51A9Tp93nxDpIRjVl9x/dpP2cDgPxEZw jkPZvUKv28H3BL0S35Y0UTCej2uVenT+ej+ilLXrhg1qg47jyl6IEQn7yK4OewMRf3d5 MWqsPL80mvKNhm5Xb6hfUMwHlJITljehsGcxc+zyPL/7QfaETLOB9wBLc0tL1yQb9isa 37Pw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1679134558; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=fHjSWW/YZEGEGz44nnM9t/++2Yb+yS/AmGPiI8kwmpU=; b=Y8FZsWQr2twNMr/LFSytaf/dH230FGv4BSzxN2dXPM4ys1wYRXcse9PjND3jGIaw/T IaCCN7x0uYIKQwVd7eQA0JWzIDzBXqBFDcQnvbXWEsYzK5K3KE0HyqI7DuMJmZ0SoOTX zXNbZoyAYuGoYFSD+xOpfZeQcSN4a7shhofFPpnIg74BB3My0mhPMI/ttoAP4eOLOHKy 2cJn8Rhd8CxGyvgpkg2m3pCuhRsX3Em3qIbIN+uHSYJIkuZUhDjcp02WpH130RnXrine cOSf0i526m+g85+sIJGKdo5JqK/cTULNGMvZGNT24EA3INEHJOqKVrmI5xs/ZFUMrudY xe2w== X-Gm-Message-State: AO0yUKVNXYvoTsYwEi4caBg6lK/Nbg0e1dFDVCFx3ML7WD70CP+poc/z x03hEJiGgdLAlnQYW258EQoDlsEwUU4= X-Google-Smtp-Source: AK7set/8cGeWnyT/kjwQEnZZrYLFcw5MaspMjNHWhsV5r485by8ve6ax7iyRNb9YE5SGrcK1K7PS7Q== X-Received: by 2002:a05:600c:548d:b0:3eb:2da5:e19 with SMTP id iv13-20020a05600c548d00b003eb2da50e19mr27182285wmb.27.1679134558462; Sat, 18 Mar 2023 03:15:58 -0700 (PDT) Received: from amir-ThinkPad-T480.lan ([5.29.249.86]) by smtp.gmail.com with ESMTPSA id v26-20020a05600c215a00b003eafc47eb09sm4333965wml.43.2023.03.18.03.15.57 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 18 Mar 2023 03:15:58 -0700 (PDT) From: Amir Goldstein To: Greg Kroah-Hartman Cc: Sasha Levin , "Darrick J . Wong" , Leah Rumancik , Chandan Babu R , Christian Brauner , linux-fsdevel@vger.kernel.org, linux-xfs@vger.kernel.org, stable@vger.kernel.org, Miklos Szeredi Subject: [PATCH 5.10 14/15] fs: use consistent setgid checks in is_sxid() Date: Sat, 18 Mar 2023 12:15:28 +0200 Message-Id: <20230318101529.1361673-15-amir73il@gmail.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20230318101529.1361673-1-amir73il@gmail.com> References: <20230318101529.1361673-1-amir73il@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org From: Christian Brauner commit 8d84e39d76bd83474b26cb44f4b338635676e7e8 upstream. Now that we made the VFS setgid checking consistent an inode can't be marked security irrelevant even if the setgid bit is still set. Make this function consistent with all other helpers. Note that enforcing consistent setgid stripping checks for file modification and mode- and ownership changes will cause the setgid bit to be lost in more cases than useed to be the case. If an unprivileged user wrote to a non-executable setgid file that they don't have privilege over the setgid bit will be dropped. This will lead to temporary failures in some xfstests until they have been updated. Reported-by: Miklos Szeredi Signed-off-by: Christian Brauner (Microsoft) Signed-off-by: Amir Goldstein --- include/linux/fs.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/linux/fs.h b/include/linux/fs.h index 57afa4fa5e7b..8ce9e5c61ede 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -3408,7 +3408,7 @@ int __init get_filesystem_list(char *buf); static inline bool is_sxid(umode_t mode) { - return (mode & S_ISUID) || ((mode & S_ISGID) && (mode & S_IXGRP)); + return mode & (S_ISUID | S_ISGID); } static inline int check_sticky(struct inode *dir, struct inode *inode) From patchwork Sat Mar 18 10:15:29 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Amir Goldstein X-Patchwork-Id: 13179742 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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 42BE4C61DA4 for ; Sat, 18 Mar 2023 10:16:26 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230034AbjCRKQZ (ORCPT ); Sat, 18 Mar 2023 06:16:25 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55350 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229972AbjCRKQD (ORCPT ); Sat, 18 Mar 2023 06:16:03 -0400 Received: from mail-wm1-x331.google.com (mail-wm1-x331.google.com [IPv6:2a00:1450:4864:20::331]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E4DD03C783; Sat, 18 Mar 2023 03:16:00 -0700 (PDT) Received: by mail-wm1-x331.google.com with SMTP id o11-20020a05600c4fcb00b003eb33ea29a8so4726639wmq.1; Sat, 18 Mar 2023 03:16:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1679134560; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=Wp5O0Uso/5747JI3g/VP4quGRfaMOOJgNJaJo0zDdP4=; b=Ar7e1HbyfilIUQ2zBdtghWRaWA+mJsxW3OOnYY9TOMxsxA8dEVH9SAKk/YmpYShPR7 CwLCRy4uxRS/QKOCwXr1r5zryMnBMZv9J0E1zBERHzdWjIjO5H/sc9ES9RcEMlmAAVw3 SOIUG2XLJsDb5i/ZTZRSQuHzwtRn4AJMzeJo0d16gGiDbNbOE7lUGphzqWnOu5l8u3K6 BFsLFvM/yWMu+uZiAPB9xmmhD1VH2AgS1it0HdIULiORfGHQuDK8mjEEUbG2UnvnqE0f r1qmM+pU1SK7dqcx2V+S3JXyxr16IInnMt/gqhW/y3fb49qV0sY1hXEk00x2waMF/76Q CFBA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1679134560; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=Wp5O0Uso/5747JI3g/VP4quGRfaMOOJgNJaJo0zDdP4=; b=mthmaI9Hw7XlJbxHSV10h4gV7qWwi/l53QYRKuLzlPdvjUjlloT6KXbaoUVG4Eoz+n Jl/S9TEZq9EICCjX138G5JOPSnJKTBSumrx++rM84tOXfPZXRhxUyqETDR8iMKMgsrrr eTW/SLAwOuucazzOEw8bQ+rlZyAuu47g7kgYe7yYNY1473nSWoYXlSPBRYsEMmn9tBO+ ZPKjMVCIseeIlS1Sd8UjGVf+6i3wVzCd8ayG3mAFUndsFSX3lZjL6mVjuPN8nZoBqlFO dyuRHc2GGBIIYSo0JQ55bPsKfDIcrm2Tv7HaJhy2bCm5UcaIIt+/EWiH5E7t6VeU5Kkw 5zkw== X-Gm-Message-State: AO0yUKWwkR4eUi7X7Aehi32xYnx2aq3wRM5f3J5mC9jkkWbr7WoXNXzu LCXipHIuhW1UYwfnOwHc9IY= X-Google-Smtp-Source: AK7set8+NSpt4bUY/HtBnBhhL4uLbkrDAtl6P2ijPkVSNV8icwmsEzLKoI97DDA8GYyjBtuf9x97XA== X-Received: by 2002:a1c:4c02:0:b0:3ed:6c71:9dc8 with SMTP id z2-20020a1c4c02000000b003ed6c719dc8mr6965394wmf.22.1679134560222; Sat, 18 Mar 2023 03:16:00 -0700 (PDT) Received: from amir-ThinkPad-T480.lan ([5.29.249.86]) by smtp.gmail.com with ESMTPSA id v26-20020a05600c215a00b003eafc47eb09sm4333965wml.43.2023.03.18.03.15.58 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 18 Mar 2023 03:15:59 -0700 (PDT) From: Amir Goldstein To: Greg Kroah-Hartman Cc: Sasha Levin , "Darrick J . Wong" , Leah Rumancik , Chandan Babu R , Christian Brauner , linux-fsdevel@vger.kernel.org, linux-xfs@vger.kernel.org, stable@vger.kernel.org, Gaosheng Cui , Carlos Maiolino , Dave Chinner Subject: [PATCH 5.10 15/15] xfs: remove xfs_setattr_time() declaration Date: Sat, 18 Mar 2023 12:15:29 +0200 Message-Id: <20230318101529.1361673-16-amir73il@gmail.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20230318101529.1361673-1-amir73il@gmail.com> References: <20230318101529.1361673-1-amir73il@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org From: Gaosheng Cui commit b0463b9dd7030a766133ad2f1571f97f204d7bdf upstream. xfs_setattr_time() has been removed since commit e014f37db1a2 ("xfs: use setattr_copy to set vfs inode attributes"), so remove it. Signed-off-by: Gaosheng Cui Reviewed-by: Carlos Maiolino Signed-off-by: Dave Chinner Signed-off-by: Amir Goldstein --- fs/xfs/xfs_iops.h | 1 - 1 file changed, 1 deletion(-) diff --git a/fs/xfs/xfs_iops.h b/fs/xfs/xfs_iops.h index 4d24ff309f59..dd1bd0332f8e 100644 --- a/fs/xfs/xfs_iops.h +++ b/fs/xfs/xfs_iops.h @@ -18,7 +18,6 @@ extern ssize_t xfs_vn_listxattr(struct dentry *, char *data, size_t size); */ #define XFS_ATTR_NOACL 0x01 /* Don't call posix_acl_chmod */ -extern void xfs_setattr_time(struct xfs_inode *ip, struct iattr *iattr); extern int xfs_setattr_nonsize(struct xfs_inode *ip, struct iattr *vap, int flags); extern int xfs_vn_setattr_nonsize(struct dentry *dentry, struct iattr *vap);