From patchwork Thu Sep 22 15:47:27 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Amir Goldstein X-Patchwork-Id: 12985471 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 D3861C6FA82 for ; Thu, 22 Sep 2022 15:47:42 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231715AbiIVPrl (ORCPT ); Thu, 22 Sep 2022 11:47:41 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:60992 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231613AbiIVPrj (ORCPT ); Thu, 22 Sep 2022 11:47:39 -0400 Received: from mail-wr1-x434.google.com (mail-wr1-x434.google.com [IPv6:2a00:1450:4864:20::434]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 4991DD4AB8; Thu, 22 Sep 2022 08:47:38 -0700 (PDT) Received: by mail-wr1-x434.google.com with SMTP id g3so16152827wrq.13; Thu, 22 Sep 2022 08:47:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date; bh=esm7WY5NBPF0ohza2iIRHTShjFzVuQEeXBALGm/uCz0=; b=MyLwk/SLFuOXKgvB8C5O1u2jCAY+nGX2svUIJEQkDE2zJogq1pGm4wN2wWMsQqFmw8 9AVfevYUqvm0zsB+x6y5j9ZmJZFKwbVTfjKi8Zwtchw/Jk8y8CEtsesIdi4jkuCW5PO9 a2Aiota+7TncWV+4tbW8MtF546Vg8Zg01nKKAa3iEoWdrs9b6jYigTGenlYaTtWbQpYs qUcu28+YTSNvHsPJU+gVtmeRdjyy9DKciQXif4bBYiwEFpS2Q1qLnDgKHVRMMAMpfHC8 3GZQzQd6z98Ke7zxcHmnerk8wdVk/WCwXDvrXQCfTnBGKB2eSoS4kOkWR8iq0sp3OC1k nwzA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; 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; bh=esm7WY5NBPF0ohza2iIRHTShjFzVuQEeXBALGm/uCz0=; b=awXjA73tO+8qnCnaG75XpKP3R5uc7M9Y6aMWvqH7akLKaXj7/pnLLANno5fARppW/f MixWy6r/wYqw/IGMZhTFWnDI/5Xbdwtqhg15JhzbAnJk9wFt49yQYi95indap6Ls4PQa oQaIsR67v3A8WIV2SB4mMeN1Ij2tbrOCHYzueOfQd0kHzXebrTYiq2eHzxn9jKenF0K7 VG8RiiBRAUxcXHrg7ozP1+oX83BTbdKYXyecdyd7P6kmBK3AWDRE0ihnUmk+sChrF/XP d/t0521V00leJhveXTRpa4qAwxyPT+PBkLqSO2xMoH4a5MubzxzHsPLt/zW6Mh6q0jPB U2Qg== X-Gm-Message-State: ACrzQf1DofNmxheOUGROYTwnxit50o3m9nkRZ/98i0di9VOWmC4TFfVh fhxiA995HfxPWrU0RnKSSmU= X-Google-Smtp-Source: AMsMyM4l3WSuMnmkT345QdBeqMLVAjYxVv7j6L2I9vAsNxcZo9Fxx9ZqY92LQWuhMbmPBh0MgnPjoA== X-Received: by 2002:a05:6000:178e:b0:22b:451:9f63 with SMTP id e14-20020a056000178e00b0022b04519f63mr2418147wrg.521.1663861656557; Thu, 22 Sep 2022 08:47:36 -0700 (PDT) Received: from amir-ThinkPad-T480.lan ([5.29.8.191]) by smtp.gmail.com with ESMTPSA id z5-20020a5d6405000000b0022af9555669sm6272386wru.99.2022.09.22.08.47.35 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 22 Sep 2022 08:47:36 -0700 (PDT) From: Amir Goldstein To: Greg Kroah-Hartman Cc: Sasha Levin , "Darrick J . Wong" , Leah Rumancik , Chandan Babu R , linux-xfs@vger.kernel.org, stable@vger.kernel.org, Dave Chinner , Frank Hofmann , "Darrick J . Wong" , Dave Chinner Subject: [PATCH 5.10 1/2] xfs: reorder iunlink remove operation in xfs_ifree Date: Thu, 22 Sep 2022 18:47:27 +0300 Message-Id: <20220922154728.97402-2-amir73il@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20220922154728.97402-1-amir73il@gmail.com> References: <20220922154728.97402-1-amir73il@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org From: Dave Chinner commit 9a5280b312e2e7898b6397b2ca3cfd03f67d7be1 upstream. [backport for 5.10.y] The O_TMPFILE creation implementation creates a specific order of operations for inode allocation/freeing and unlinked list modification. Currently both are serialised by the AGI, so the order doesn't strictly matter as long as the are both in the same transaction. However, if we want to move the unlinked list insertions largely out from under the AGI lock, then we have to be concerned about the order in which we do unlinked list modification operations. O_TMPFILE creation tells us this order is inode allocation/free, then unlinked list modification. Change xfs_ifree() to use this same ordering on unlinked list removal. This way we always guarantee that when we enter the iunlinked list removal code from this path, we already have the AGI locked and we don't have to worry about lock nesting AGI reads inside unlink list locks because it's already locked and attached to the transaction. We can do this safely as the inode freeing and unlinked list removal are done in the same transaction and hence are atomic operations with respect to log recovery. Reported-by: Frank Hofmann Fixes: 298f7bec503f ("xfs: pin inode backing buffer to the inode log item") Signed-off-by: Dave Chinner Reviewed-by: Darrick J. Wong Signed-off-by: Dave Chinner Signed-off-by: Amir Goldstein Acked-by: Darrick J. Wong --- fs/xfs/xfs_inode.c | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c index 1f61e085676b..929ed3bc5619 100644 --- a/fs/xfs/xfs_inode.c +++ b/fs/xfs/xfs_inode.c @@ -2669,14 +2669,13 @@ xfs_ifree_cluster( } /* - * This is called to return an inode to the inode free list. - * The inode should already be truncated to 0 length and have - * no pages associated with it. This routine also assumes that - * the inode is already a part of the transaction. + * This is called to return an inode to the inode free list. The inode should + * already be truncated to 0 length and have no pages associated with it. This + * routine also assumes that the inode is already a part of the transaction. * - * The on-disk copy of the inode will have been added to the list - * of unlinked inodes in the AGI. We need to remove the inode from - * that list atomically with respect to freeing it here. + * The on-disk copy of the inode will have been added to the list of unlinked + * inodes in the AGI. We need to remove the inode from that list atomically with + * respect to freeing it here. */ int xfs_ifree( @@ -2694,13 +2693,16 @@ xfs_ifree( ASSERT(ip->i_d.di_nblocks == 0); /* - * Pull the on-disk inode from the AGI unlinked list. + * Free the inode first so that we guarantee that the AGI lock is going + * to be taken before we remove the inode from the unlinked list. This + * makes the AGI lock -> unlinked list modification order the same as + * used in O_TMPFILE creation. */ - error = xfs_iunlink_remove(tp, ip); + error = xfs_difree(tp, ip->i_ino, &xic); if (error) return error; - error = xfs_difree(tp, ip->i_ino, &xic); + error = xfs_iunlink_remove(tp, ip); if (error) return error; From patchwork Thu Sep 22 15:47:28 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Amir Goldstein X-Patchwork-Id: 12985472 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 4F8DBC6FA92 for ; Thu, 22 Sep 2022 15:47:44 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231615AbiIVPrm (ORCPT ); Thu, 22 Sep 2022 11:47:42 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:32882 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231678AbiIVPrk (ORCPT ); Thu, 22 Sep 2022 11:47:40 -0400 Received: from mail-wr1-x42c.google.com (mail-wr1-x42c.google.com [IPv6:2a00:1450:4864:20::42c]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id BE72FD62C9; Thu, 22 Sep 2022 08:47:39 -0700 (PDT) Received: by mail-wr1-x42c.google.com with SMTP id x18so10201751wrm.7; Thu, 22 Sep 2022 08:47:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date; bh=M3jAT3YQCl4NufeW6oCkMl9aVNOp6TjzxP0+mQ9Dows=; b=jbN4VNQS1GLIDFf5MK+GytMiPVDyIaatKirmfHc7QwA0Biqs4fJGYOWXWvMd24xUHn qklGBwV2/K4RU03rVfTrb5CWE6X61d9FwhGBTVOfMfEf4DlOUKQsUUItjs23/LzTdJsx r/WA5Vl83DWF3vUAmErPKFcsSjehiSw5wIdlkBSL8A3S4xzOPk4r9EbyTsGvUTUm4wCA 8AunZsZTmLE+ya+akHY4n0fgEaSj+XUj1vtn+Ht6UmEcLiw1JwFa40gQAdhN9CD/woN8 xFtDKwdBDEzSY5pNfgi2L9XNoJ+yOx4SrZrm5zlkXVAISPtb4SO8GmAue2TZKlH4XZC5 LbDg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; 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; bh=M3jAT3YQCl4NufeW6oCkMl9aVNOp6TjzxP0+mQ9Dows=; b=IYR6s63yGw4AOYPql5f1F06YGhgw0nI1qt9G0ClV7/6GKM/nqHD7eiKQG7bhNI8XsN LJ/L30zhKzXuKNLt/HadUjRBwflZZSWv4vbHs6qqkWPJ4VEEChhZ16w9TmXJsBrYivA6 ZN3nT1awS34UdXaDn1yNRwUyS9n6sEcRxmI1sRM48Z6Se+iNoqGIqAWuqXPqK3riC0xO Pe6tzup8dbvLlXoZ2ZIW9uiheJRsBs/HSTOGUNXcEmuAecyYC/jFC/IDKMSEVQ0shZAt 4YoHkBluSNJPrTFov55ellpfZMi3CHknEe3y9No/aTMJLrSrukDwGqKGGtcKLH+wq0iM iUlQ== X-Gm-Message-State: ACrzQf18gI30Zf0GqjaRxETFvZevtcTcN5SbkU86RkzA0L7dWJIt62JH QEKMnME9KEME3vBfE8REiLI= X-Google-Smtp-Source: AMsMyM4YHhvTkXol4aB/0ol8N1rHVk04QRHWTntg+wOMynfpirRltShpG7OgNP+23cQ/HAwhET5KLw== X-Received: by 2002:a5d:64ab:0:b0:226:d997:ad5c with SMTP id m11-20020a5d64ab000000b00226d997ad5cmr2432690wrp.602.1663861658164; Thu, 22 Sep 2022 08:47:38 -0700 (PDT) Received: from amir-ThinkPad-T480.lan ([5.29.8.191]) by smtp.gmail.com with ESMTPSA id z5-20020a5d6405000000b0022af9555669sm6272386wru.99.2022.09.22.08.47.36 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 22 Sep 2022 08:47:37 -0700 (PDT) From: Amir Goldstein To: Greg Kroah-Hartman Cc: Sasha Levin , "Darrick J . Wong" , Leah Rumancik , Chandan Babu R , linux-xfs@vger.kernel.org, stable@vger.kernel.org, Dave Chinner , Christoph Hellwig , Dave Chinner Subject: [PATCH 5.10 2/2] xfs: validate inode fork size against fork format Date: Thu, 22 Sep 2022 18:47:28 +0300 Message-Id: <20220922154728.97402-3-amir73il@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20220922154728.97402-1-amir73il@gmail.com> References: <20220922154728.97402-1-amir73il@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org From: Dave Chinner commit 1eb70f54c445fcbb25817841e774adb3d912f3e8 upstream. [backport for 5.10.y] xfs_repair catches fork size/format mismatches, but the in-kernel verifier doesn't, leading to null pointer failures when attempting to perform operations on the fork. This can occur in the xfs_dir_is_empty() where the in-memory fork format does not match the size and so the fork data pointer is accessed incorrectly. Note: this causes new failures in xfs/348 which is testing mode vs ftype mismatches. We now detect a regular file that has been changed to a directory or symlink mode as being corrupt because the data fork is for a symlink or directory should be in local form when there are only 3 bytes of data in the data fork. Hence the inode verify for the regular file now fires w/ -EFSCORRUPTED because the inode fork format does not match the format the corrupted mode says it should be in. Signed-off-by: Dave Chinner Reviewed-by: Christoph Hellwig Reviewed-by: Darrick J. Wong Signed-off-by: Dave Chinner Signed-off-by: Amir Goldstein Acked-by: Darrick J. Wong --- fs/xfs/libxfs/xfs_inode_buf.c | 35 ++++++++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c index c667c63f2cb0..fa8aefe6b7ec 100644 --- a/fs/xfs/libxfs/xfs_inode_buf.c +++ b/fs/xfs/libxfs/xfs_inode_buf.c @@ -358,19 +358,36 @@ xfs_dinode_verify_fork( int whichfork) { uint32_t di_nextents = XFS_DFORK_NEXTENTS(dip, whichfork); + mode_t mode = be16_to_cpu(dip->di_mode); + uint32_t fork_size = XFS_DFORK_SIZE(dip, mp, whichfork); + uint32_t fork_format = XFS_DFORK_FORMAT(dip, whichfork); - switch (XFS_DFORK_FORMAT(dip, whichfork)) { + /* + * For fork types that can contain local data, check that the fork + * format matches the size of local data contained within the fork. + * + * For all types, check that when the size says the should be in extent + * or btree format, the inode isn't claiming it is in local format. + */ + if (whichfork == XFS_DATA_FORK) { + if (S_ISDIR(mode) || S_ISLNK(mode)) { + if (be64_to_cpu(dip->di_size) <= fork_size && + fork_format != XFS_DINODE_FMT_LOCAL) + return __this_address; + } + + if (be64_to_cpu(dip->di_size) > fork_size && + fork_format == XFS_DINODE_FMT_LOCAL) + return __this_address; + } + + switch (fork_format) { case XFS_DINODE_FMT_LOCAL: /* - * no local regular files yet + * No local regular files yet. */ - if (whichfork == XFS_DATA_FORK) { - if (S_ISREG(be16_to_cpu(dip->di_mode))) - return __this_address; - if (be64_to_cpu(dip->di_size) > - XFS_DFORK_SIZE(dip, mp, whichfork)) - return __this_address; - } + if (S_ISREG(mode) && whichfork == XFS_DATA_FORK) + return __this_address; if (di_nextents) return __this_address; break;