From patchwork Thu Feb 16 20:49:52 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Darrick J. Wong" X-Patchwork-Id: 13143844 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 1A152C61DA4 for ; Thu, 16 Feb 2023 20:49:59 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229747AbjBPUt5 (ORCPT ); Thu, 16 Feb 2023 15:49:57 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:58598 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229744AbjBPUt5 (ORCPT ); Thu, 16 Feb 2023 15:49:57 -0500 Received: from ams.source.kernel.org (ams.source.kernel.org [IPv6:2604:1380:4601:e00::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 85CBE4BEB7 for ; Thu, 16 Feb 2023 12:49:55 -0800 (PST) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id 11FCEB82958 for ; Thu, 16 Feb 2023 20:49:54 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id C8F5FC433D2; Thu, 16 Feb 2023 20:49:52 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1676580592; bh=nU8MZIkA/59rap4/jMUE8cSiAeB2ujtnNvbww7B9t+k=; h=Date:Subject:From:To:Cc:In-Reply-To:References:From; b=jqRePH0iyjc3P1S1MD+f1SSiMU3mfUMumPCRlyxQVNM4SYGHA6Q9khOND00B+jpMm m3h0Ber0bgUbzC34Ynwt8H5j0Nd4au8dBR5FeZmLpv7DdG16jQJF2/xGVAREyYqsJV QybS7judzxB4CZ1mqjJN2UURm1jZczTLHpc985ebcVsc9CaHwdX8q5py//kCwaEoSY EVxqjcOSrMqlyy9gr9rO8PnMhr1LKfw1zJCO5n0d2cyt3s+rBWedCwrH0Pqtj5jNkx D+c64YtUwOGpJ2wlwRf64Wm/m0qegQQzb6UOBElnuvSoRCeQSPeTbvHAZuo01Vxdjn PhVWO6FUQtC4g== Date: Thu, 16 Feb 2023 12:49:52 -0800 Subject: [PATCH 1/2] xfs: scrub parent pointers From: "Darrick J. Wong" To: djwong@kernel.org Cc: allison.henderson@oracle.com, linux-xfs@vger.kernel.org Message-ID: <167657874879.3475106.6130523299352387443.stgit@magnolia> In-Reply-To: <167657874864.3475106.13930268587808485264.stgit@magnolia> References: <167657874864.3475106.13930268587808485264.stgit@magnolia> User-Agent: StGit/0.19 MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org From: Darrick J. Wong Actually check parent pointers now. Signed-off-by: Darrick J. Wong --- fs/xfs/scrub/parent.c | 291 +++++++++++++++++++++++++++++++++++++++++++++++++ fs/xfs/scrub/trace.h | 33 ++++++ 2 files changed, 324 insertions(+) diff --git a/fs/xfs/scrub/parent.c b/fs/xfs/scrub/parent.c index d59184a59671..1bb196f2c1b2 100644 --- a/fs/xfs/scrub/parent.c +++ b/fs/xfs/scrub/parent.c @@ -14,9 +14,13 @@ #include "xfs_icache.h" #include "xfs_dir2.h" #include "xfs_dir2_priv.h" +#include "xfs_attr.h" +#include "xfs_parent.h" #include "scrub/scrub.h" #include "scrub/common.h" #include "scrub/readdir.h" +#include "scrub/listxattr.h" +#include "scrub/trace.h" /* Set us up to scrub parents. */ int @@ -231,6 +235,8 @@ xchk_parent_validate( return error; } +STATIC int xchk_parent_pptr(struct xfs_scrub *sc); + /* Scrub a parent pointer. */ int xchk_parent( @@ -240,6 +246,9 @@ xchk_parent( xfs_ino_t parent_ino; int error; + if (xfs_has_parent(mp)) + return xchk_parent_pptr(sc); + /* * If we're a directory, check that the '..' link points up to * a directory that has one entry pointing to us. @@ -282,3 +291,285 @@ xchk_parent( return xchk_parent_validate(sc, parent_ino); } + +/* + * Checking of Parent Pointers + * =========================== + * + * On filesystems with directory parent pointers, we check the referential + * integrity by visiting each parent pointer of a child file and checking that + * the directory referenced by the pointer actually has a dirent pointing back + * to the child file. + */ + +struct xchk_pptrs { + struct xfs_scrub *sc; + + /* Scratch buffer for scanning pptr xattrs */ + struct xfs_parent_name_irec pptr; + + /* Parent of this directory. */ + xfs_ino_t parent_ino; +}; + +/* Look up the dotdot entry so that we can check it as we walk the pptrs. */ +STATIC int +xchk_parent_dotdot( + struct xchk_pptrs *pp) +{ + struct xfs_scrub *sc = pp->sc; + int error; + + if (!S_ISDIR(VFS_I(sc->ip)->i_mode)) { + pp->parent_ino = NULLFSINO; + return 0; + } + + /* Look up '..' */ + error = xchk_dir_lookup(sc, sc->ip, &xfs_name_dotdot, &pp->parent_ino, + NULL); + if (!xchk_fblock_process_error(sc, XFS_DATA_FORK, 0, &error)) + return error; + if (!xfs_verify_dir_ino(sc->mp, pp->parent_ino)) { + xchk_fblock_set_corrupt(sc, XFS_DATA_FORK, 0); + return 0; + } + + /* Is this the root dir? Then '..' must point to itself. */ + if (sc->ip == sc->mp->m_rootip && sc->ip->i_ino != pp->parent_ino) + xchk_fblock_set_corrupt(sc, XFS_DATA_FORK, 0); + + return 0; +} + +/* + * Try to lock a parent directory for checking dirents. Returns the inode + * flags for the locks we now hold, or zero if we failed. + */ +STATIC unsigned int +xchk_parent_lock_dir( + struct xfs_scrub *sc, + struct xfs_inode *dp) +{ + if (!xfs_ilock_nowait(dp, XFS_IOLOCK_SHARED)) + return 0; + + if (!xfs_ilock_nowait(dp, XFS_ILOCK_SHARED)) { + xfs_iunlock(dp, XFS_IOLOCK_SHARED); + return 0; + } + + if (!xfs_need_iread_extents(&dp->i_df)) + return XFS_IOLOCK_SHARED | XFS_ILOCK_SHARED; + + xfs_iunlock(dp, XFS_ILOCK_SHARED); + + if (!xfs_ilock_nowait(dp, XFS_ILOCK_EXCL)) { + xfs_iunlock(dp, XFS_IOLOCK_SHARED); + return 0; + } + + return XFS_IOLOCK_SHARED | XFS_ILOCK_EXCL; +} + +/* Check the forward link (dirent) associated with this parent pointer. */ +STATIC int +xchk_parent_dirent( + struct xchk_pptrs *pp, + struct xfs_inode *dp) +{ + struct xfs_name xname = { + .name = pp->pptr.p_name, + .len = pp->pptr.p_namelen, + }; + struct xfs_scrub *sc = pp->sc; + xfs_ino_t child_ino; + xfs_dir2_dataptr_t child_diroffset; + int error; + + /* + * Use the name attached to this parent pointer to look up the + * directory entry in the alleged parent. + */ + error = xchk_dir_lookup(sc, dp, &xname, &child_ino, &child_diroffset); + if (error == -ENOENT) { + xchk_fblock_xref_set_corrupt(sc, XFS_ATTR_FORK, 0); + return 0; + } + if (!xchk_fblock_xref_process_error(sc, XFS_ATTR_FORK, 0, &error)) + return error; + + /* Does the inode number match? */ + if (child_ino != sc->ip->i_ino) { + xchk_fblock_xref_set_corrupt(sc, XFS_ATTR_FORK, 0); + return 0; + } + + /* Does the directory offset match? */ + if (pp->pptr.p_diroffset != child_diroffset) { + trace_xchk_parent_bad_dapos(sc->ip, pp->pptr.p_diroffset, + dp->i_ino, child_diroffset, xname.name, + xname.len); + xchk_fblock_xref_set_corrupt(sc, XFS_ATTR_FORK, 0); + return 0; + } + + /* + * If we're scanning a directory, we should only ever encounter a + * single parent pointer, and it should match the dotdot entry. We set + * the parent_ino from the dotdot entry before the scan, so compare it + * now. + */ + if (!S_ISDIR(VFS_I(sc->ip)->i_mode)) + return 0; + + if (pp->parent_ino != dp->i_ino) { + xchk_fblock_set_corrupt(sc, XFS_ATTR_FORK, 0); + return 0; + } + + pp->parent_ino = NULLFSINO; + return 0; +} + +/* Try to grab a parent directory. */ +STATIC int +xchk_parent_iget( + struct xchk_pptrs *pp, + struct xfs_inode **dpp) +{ + struct xfs_scrub *sc = pp->sc; + struct xfs_inode *ip; + int error; + + /* Validate inode number. */ + error = xfs_dir_ino_validate(sc->mp, pp->pptr.p_ino); + if (error) { + xchk_fblock_set_corrupt(sc, XFS_ATTR_FORK, 0); + return -ECANCELED; + } + + error = xchk_iget(sc, pp->pptr.p_ino, &ip); + if (error == -EINVAL || error == -ENOENT) { + xchk_fblock_set_corrupt(sc, XFS_ATTR_FORK, 0); + return -ECANCELED; + } + if (!xchk_fblock_xref_process_error(sc, XFS_ATTR_FORK, 0, &error)) + return error; + + /* The parent must be a directory. */ + if (!S_ISDIR(VFS_I(ip)->i_mode)) { + xchk_fblock_xref_set_corrupt(sc, XFS_ATTR_FORK, 0); + goto out_rele; + } + + /* Validate generation number. */ + if (VFS_I(ip)->i_generation != pp->pptr.p_gen) { + xchk_fblock_xref_set_corrupt(sc, XFS_ATTR_FORK, 0); + goto out_rele; + } + + *dpp = ip; + return 0; +out_rele: + xchk_irele(sc, ip); + return 0; +} + +/* + * Walk an xattr of a file. If this xattr is a parent pointer, follow it up + * to a parent directory and check that the parent has a dirent pointing back + * to us. + */ +STATIC int +xchk_parent_scan_attr( + struct xfs_scrub *sc, + struct xfs_inode *ip, + unsigned int attr_flags, + const unsigned char *name, + unsigned int namelen, + const void *value, + unsigned int valuelen, + void *priv) +{ + struct xchk_pptrs *pp = priv; + struct xfs_inode *dp = NULL; + const struct xfs_parent_name_rec *rec = (const void *)name; + unsigned int lockmode; + int error; + + /* Ignore incomplete xattrs */ + if (attr_flags & XFS_ATTR_INCOMPLETE) + return 0; + + /* Ignore anything that isn't a parent pointer. */ + if (!(attr_flags & XFS_ATTR_PARENT)) + return 0; + + /* Does the ondisk parent pointer structure make sense? */ + if (!xfs_parent_namecheck(sc->mp, rec, namelen, attr_flags)) { + xchk_fblock_set_corrupt(sc, XFS_ATTR_FORK, 0); + return -ECANCELED; + } + + if (!xfs_parent_valuecheck(sc->mp, value, valuelen)) { + xchk_fblock_set_corrupt(sc, XFS_ATTR_FORK, 0); + return -ECANCELED; + } + + xfs_parent_irec_from_disk(&pp->pptr, rec, value, valuelen); + + error = xchk_parent_iget(pp, &dp); + if (error) + return error; + if (!dp) + return 0; + + /* Try to lock the inode. */ + lockmode = xchk_parent_lock_dir(sc, dp); + if (!lockmode) { + xchk_set_incomplete(sc); + error = -ECANCELED; + goto out_rele; + } + + error = xchk_parent_dirent(pp, dp); + if (error) + goto out_unlock; + +out_unlock: + xfs_iunlock(dp, lockmode); +out_rele: + xchk_irele(sc, dp); + return error; +} + +/* Check parent pointers of a file. */ +STATIC int +xchk_parent_pptr( + struct xfs_scrub *sc) +{ + struct xchk_pptrs *pp; + int error; + + pp = kvzalloc(sizeof(struct xchk_pptrs), XCHK_GFP_FLAGS); + if (!pp) + return -ENOMEM; + pp->sc = sc; + + error = xchk_parent_dotdot(pp); + if (error) + goto out_pp; + + error = xchk_xattr_walk(sc, sc->ip, xchk_parent_scan_attr, pp); + if (error == -ECANCELED) { + error = 0; + goto out_pp; + } + if (error) + goto out_pp; + +out_pp: + kvfree(pp); + return error; +} diff --git a/fs/xfs/scrub/trace.h b/fs/xfs/scrub/trace.h index 81d26be0ef3b..ac21759fc3e1 100644 --- a/fs/xfs/scrub/trace.h +++ b/fs/xfs/scrub/trace.h @@ -896,6 +896,39 @@ TRACE_EVENT(xchk_nlinks_live_update, __get_str(name)) ); +TRACE_EVENT(xchk_parent_bad_dapos, + TP_PROTO(struct xfs_inode *ip, unsigned int p_diroffset, + xfs_ino_t parent_ino, unsigned int dapos, + const char *name, unsigned int namelen), + TP_ARGS(ip, p_diroffset, parent_ino, dapos, name, namelen), + TP_STRUCT__entry( + __field(dev_t, dev) + __field(xfs_ino_t, ino) + __field(unsigned int, p_diroffset) + __field(xfs_ino_t, parent_ino) + __field(unsigned int, dapos) + __field(unsigned int, namelen) + __dynamic_array(char, name, namelen) + ), + TP_fast_assign( + __entry->dev = ip->i_mount->m_super->s_dev; + __entry->ino = ip->i_ino; + __entry->p_diroffset = p_diroffset; + __entry->parent_ino = parent_ino; + __entry->dapos = dapos; + __entry->namelen = namelen; + memcpy(__get_str(name), name, namelen); + ), + TP_printk("dev %d:%d ino 0x%llx p_diroff 0x%x parent_ino 0x%llx parent_diroff 0x%x name '%.*s'", + MAJOR(__entry->dev), MINOR(__entry->dev), + __entry->ino, + __entry->p_diroffset, + __entry->parent_ino, + __entry->dapos, + __entry->namelen, + __get_str(name)) +); + /* repair tracepoints */ #if IS_ENABLED(CONFIG_XFS_ONLINE_REPAIR) From patchwork Thu Feb 16 20:50:07 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Darrick J. Wong" X-Patchwork-Id: 13143845 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 B099DC61DA4 for ; Thu, 16 Feb 2023 20:50:14 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229751AbjBPUuN (ORCPT ); Thu, 16 Feb 2023 15:50:13 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:58674 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229744AbjBPUuN (ORCPT ); Thu, 16 Feb 2023 15:50:13 -0500 Received: from ams.source.kernel.org (ams.source.kernel.org [145.40.68.75]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 4F5244C3C3 for ; Thu, 16 Feb 2023 12:50:11 -0800 (PST) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id C8BBCB82962 for ; Thu, 16 Feb 2023 20:50:09 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 61CDEC433D2; Thu, 16 Feb 2023 20:50:08 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1676580608; bh=Xl3AYVRwiF4XhTBdsM73BDTXAAxCb06LL2ae0jFxmgA=; h=Date:Subject:From:To:Cc:In-Reply-To:References:From; b=qE/nbqbx2Qc0fmQWNG0i8WNouZdqm1X5CQZQQnsYdiNKVG8Ju29BiFNowrR2tKHSt 9JnXBGH2givcYH2od9+aDmQLWZ9ZqI1wh6OmaSidciS/qXulTw34mm7RCnjpT72ksK 8d1xoAbAqWl+1sI7bIO4sGYlrsmbALmzk7VJVdG8uFAVv4h39ZK8oPwidzy/LAawxN SNKVwPedmpaBd9mKUheON44bbogQ2R6ZOFo5uuwmgDt01mEPwy0hD1OT/Q6c5Wppab kXadDfMemPle2a7JabGqPnAvN5fGiGFubBBiGSKIKHoxcJVpHUzzENdCo8WyG5MoWn dX8sDChtqAqIw== Date: Thu, 16 Feb 2023 12:50:07 -0800 Subject: [PATCH 2/2] xfs: deferred scrub of parent pointers From: "Darrick J. Wong" To: djwong@kernel.org Cc: allison.henderson@oracle.com, linux-xfs@vger.kernel.org Message-ID: <167657874893.3475106.7328116715501034300.stgit@magnolia> In-Reply-To: <167657874864.3475106.13930268587808485264.stgit@magnolia> References: <167657874864.3475106.13930268587808485264.stgit@magnolia> User-Agent: StGit/0.19 MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org From: Darrick J. Wong If the trylock-based dirent check fails, retain those parent pointers and check them at the end. This may involve dropping the locks on the file being scanned, so yay. Signed-off-by: Darrick J. Wong --- fs/xfs/Makefile | 2 fs/xfs/libxfs/xfs_parent.c | 38 +++++++ fs/xfs/libxfs/xfs_parent.h | 10 ++ fs/xfs/scrub/parent.c | 246 +++++++++++++++++++++++++++++++++++++++++++- fs/xfs/scrub/trace.h | 33 ++++++ 5 files changed, 324 insertions(+), 5 deletions(-) diff --git a/fs/xfs/Makefile b/fs/xfs/Makefile index a32f6da27a86..0a908382d033 100644 --- a/fs/xfs/Makefile +++ b/fs/xfs/Makefile @@ -168,6 +168,7 @@ xfs-y += $(addprefix scrub/, \ scrub.o \ symlink.o \ xfarray.o \ + xfblob.o \ xfile.o \ ) @@ -181,7 +182,6 @@ xfs-y += $(addprefix scrub/, \ dir_repair.o \ repair.o \ tempfile.o \ - xfblob.o \ ) endif endif diff --git a/fs/xfs/libxfs/xfs_parent.c b/fs/xfs/libxfs/xfs_parent.c index fe6d4d1a7d57..36e1968337d5 100644 --- a/fs/xfs/libxfs/xfs_parent.c +++ b/fs/xfs/libxfs/xfs_parent.c @@ -298,3 +298,41 @@ xfs_pptr_calc_space_res( XFS_NEXTENTADD_SPACE_RES(mp, namelen, XFS_ATTR_FORK); } +/* + * Look up the @name associated with the parent pointer (@pptr) of @ip. Caller + * must hold at least ILOCK_SHARED. Returns the length of the dirent name, or + * a negative errno. The scratchpad need not be initialized. + */ +int +xfs_parent_lookup( + struct xfs_trans *tp, + struct xfs_inode *ip, + const struct xfs_parent_name_irec *pptr, + unsigned char *name, + unsigned int namelen, + struct xfs_parent_scratch *scr) +{ + int error; + + xfs_parent_irec_to_disk(&scr->rec, NULL, NULL, pptr); + + memset(&scr->args, 0, sizeof(struct xfs_da_args)); + scr->args.attr_filter = XFS_ATTR_PARENT; + scr->args.dp = ip; + scr->args.geo = ip->i_mount->m_attr_geo; + scr->args.name = (const unsigned char *)&scr->rec; + scr->args.namelen = sizeof(struct xfs_parent_name_rec); + scr->args.op_flags = XFS_DA_OP_OKNOENT; + scr->args.trans = tp; + scr->args.valuelen = namelen; + scr->args.value = name; + scr->args.whichfork = XFS_ATTR_FORK; + + scr->args.hashval = xfs_da_hashname(scr->args.name, scr->args.namelen); + + error = xfs_attr_get_ilocked(&scr->args); + if (error) + return error; + + return scr->args.valuelen; +} diff --git a/fs/xfs/libxfs/xfs_parent.h b/fs/xfs/libxfs/xfs_parent.h index 4eb92fb4b11b..cd1b135195a2 100644 --- a/fs/xfs/libxfs/xfs_parent.h +++ b/fs/xfs/libxfs/xfs_parent.h @@ -103,4 +103,14 @@ xfs_parent_finish( unsigned int xfs_pptr_calc_space_res(struct xfs_mount *mp, unsigned int namelen); +/* Scratchpad memory so that raw parent operations don't burn stack space. */ +struct xfs_parent_scratch { + struct xfs_parent_name_rec rec; + struct xfs_da_args args; +}; + +int xfs_parent_lookup(struct xfs_trans *tp, struct xfs_inode *ip, + const struct xfs_parent_name_irec *pptr, unsigned char *name, + unsigned int namelen, struct xfs_parent_scratch *scratch); + #endif /* __XFS_PARENT_H__ */ diff --git a/fs/xfs/scrub/parent.c b/fs/xfs/scrub/parent.c index 1bb196f2c1b2..056e11337cec 100644 --- a/fs/xfs/scrub/parent.c +++ b/fs/xfs/scrub/parent.c @@ -20,6 +20,9 @@ #include "scrub/common.h" #include "scrub/readdir.h" #include "scrub/listxattr.h" +#include "scrub/xfile.h" +#include "scrub/xfarray.h" +#include "scrub/xfblob.h" #include "scrub/trace.h" /* Set us up to scrub parents. */ @@ -302,14 +305,43 @@ xchk_parent( * to the child file. */ +/* Deferred parent pointer entry that we saved for later. */ +struct xchk_pptr { + /* Cookie for retrieval of the pptr name. */ + xfblob_cookie name_cookie; + + /* Parent pointer attr key. */ + xfs_ino_t p_ino; + uint32_t p_gen; + xfs_dir2_dataptr_t p_diroffset; + + /* Length of the pptr name. */ + uint8_t namelen; +}; + struct xchk_pptrs { struct xfs_scrub *sc; /* Scratch buffer for scanning pptr xattrs */ struct xfs_parent_name_irec pptr; + /* Fixed-size array of xchk_pptr structures. */ + struct xfarray *pptr_entries; + + /* Blobs containing parent pointer names. */ + struct xfblob *pptr_names; + /* Parent of this directory. */ xfs_ino_t parent_ino; + + /* If we've cycled the ILOCK, we must revalidate all deferred pptrs. */ + bool need_revalidate; + + /* xattr key and da args for parent pointer revalidation. */ + struct xfs_parent_scratch pptr_scratch; + + /* Name buffer for revalidation. */ + uint8_t namebuf[MAXNAMELEN]; }; /* Look up the dotdot entry so that we can check it as we walk the pptrs. */ @@ -528,8 +560,26 @@ xchk_parent_scan_attr( /* Try to lock the inode. */ lockmode = xchk_parent_lock_dir(sc, dp); if (!lockmode) { - xchk_set_incomplete(sc); - error = -ECANCELED; + struct xchk_pptr save_pp = { + .p_ino = pp->pptr.p_ino, + .p_gen = pp->pptr.p_gen, + .p_diroffset = pp->pptr.p_diroffset, + .namelen = pp->pptr.p_namelen, + }; + + /* Couldn't lock the inode, so save the pptr for later. */ + trace_xchk_parent_defer(sc->ip, pp->pptr.p_name, + pp->pptr.p_namelen, dp->i_ino); + + error = xfblob_store(pp->pptr_names, &save_pp.name_cookie, + pp->pptr.p_name, pp->pptr.p_namelen); + if (xchk_fblock_process_error(sc, XFS_ATTR_FORK, 0, &error)) + goto out_rele; + + error = xfarray_append(pp->pptr_entries, &save_pp); + if (xchk_fblock_process_error(sc, XFS_ATTR_FORK, 0, &error)) + goto out_rele; + goto out_rele; } @@ -544,6 +594,173 @@ xchk_parent_scan_attr( return error; } +/* + * Revalidate a parent pointer that we collected in the past but couldn't check + * because of lock contention. Returns 0 if the parent pointer is still valid, + * -ENOENT if it has gone away on us, or a negative errno. + */ +STATIC int +xchk_parent_revalidate_pptr( + struct xchk_pptrs *pp) +{ + struct xfs_scrub *sc = pp->sc; + int namelen; + + namelen = xfs_parent_lookup(sc->tp, sc->ip, &pp->pptr, pp->namebuf, + MAXNAMELEN, &pp->pptr_scratch); + if (namelen == -ENOATTR) { + /* Parent pointer went away, nothing to revalidate. */ + return -ENOENT; + } + if (namelen < 0 && namelen != -EEXIST) + return namelen; + + /* + * The dirent name changed length while we were unlocked. No need + * to revalidate this. + */ + if (namelen != pp->pptr.p_namelen) + return -ENOENT; + + /* The dirent name itself changed; there's nothing to revalidate. */ + if (memcmp(pp->namebuf, pp->pptr.p_name, pp->pptr.p_namelen)) + return -ENOENT; + return 0; +} + +/* + * Check a parent pointer the slow way, which means we cycle locks a bunch + * and put up with revalidation until we get it done. + */ +STATIC int +xchk_parent_slow_pptr( + struct xchk_pptrs *pp, + struct xchk_pptr *pptr) +{ + struct xfs_scrub *sc = pp->sc; + struct xfs_inode *dp = NULL; + unsigned int lockmode; + int error; + + /* Restore the saved parent pointer into the irec. */ + pp->pptr.p_ino = pptr->p_ino; + pp->pptr.p_gen = pptr->p_gen; + pp->pptr.p_diroffset = pptr->p_diroffset; + + error = xfblob_load(pp->pptr_names, pptr->name_cookie, pp->pptr.p_name, + pptr->namelen); + if (error) + return error; + pp->pptr.p_name[MAXNAMELEN - 1] = 0; + pp->pptr.p_namelen = pptr->namelen; + + /* Check that the deferred parent pointer still exists. */ + if (pp->need_revalidate) { + error = xchk_parent_revalidate_pptr(pp); + if (error == -ENOENT) + return 0; + if (!xchk_fblock_xref_process_error(sc, XFS_ATTR_FORK, 0, + &error)) + return error; + } + + error = xchk_parent_iget(pp, &dp); + if (error) + return error; + if (!dp) + return 0; + + /* + * If we can grab both IOLOCK and ILOCK of the alleged parent, we + * can proceed with the validation. + */ + lockmode = xchk_parent_lock_dir(sc, dp); + if (lockmode) + goto check_dirent; + + /* + * We couldn't lock the parent dir. Drop all the locks and try to + * get them again, one at a time. + */ + xchk_iunlock(sc, sc->ilock_flags); + pp->need_revalidate = true; + + trace_xchk_parent_slowpath(sc->ip, pp->namebuf, pptr->namelen, + dp->i_ino); + + while (true) { + xchk_ilock(sc, XFS_IOLOCK_EXCL); + if (xfs_ilock_nowait(dp, XFS_IOLOCK_SHARED)) { + xchk_ilock(sc, XFS_ILOCK_EXCL); + if (xfs_ilock_nowait(dp, XFS_ILOCK_EXCL)) { + break; + } + xchk_iunlock(sc, XFS_ILOCK_EXCL); + } + xchk_iunlock(sc, XFS_IOLOCK_EXCL); + + if (xchk_should_terminate(sc, &error)) + goto out_rele; + + delay(1); + } + lockmode = XFS_IOLOCK_SHARED | XFS_ILOCK_EXCL; + + /* + * If we didn't already find a parent pointer matching the dotdot + * entry, re-query the dotdot entry so that we can validate it. + */ + if (pp->parent_ino != NULLFSINO) { + error = xchk_parent_dotdot(pp); + if (error) + goto out_unlock; + } + + /* Revalidate the parent pointer now that we cycled locks. */ + error = xchk_parent_revalidate_pptr(pp); + if (error == -ENOENT) + goto out_unlock; + if (!xchk_fblock_xref_process_error(sc, XFS_ATTR_FORK, 0, &error)) + goto out_unlock; + +check_dirent: + error = xchk_parent_dirent(pp, dp); +out_unlock: + xfs_iunlock(dp, lockmode); +out_rele: + xchk_irele(sc, dp); + return error; +} + +/* Check all the parent pointers that we deferred the first time around. */ +STATIC int +xchk_parent_finish_slow_pptrs( + struct xchk_pptrs *pp) +{ + xfarray_idx_t array_cur; + int error; + + foreach_xfarray_idx(pp->pptr_entries, array_cur) { + struct xchk_pptr pptr; + + if (pp->sc->sm->sm_flags & XFS_SCRUB_OFLAG_CORRUPT) + return 0; + + error = xfarray_load(pp->pptr_entries, array_cur, &pptr); + if (error) + return error; + + error = xchk_parent_slow_pptr(pp, &pptr); + if (error) + return error; + } + + /* Empty out both xfiles now that we've checked everything. */ + xfarray_truncate(pp->pptr_entries); + xfblob_truncate(pp->pptr_names); + return 0; +} + /* Check parent pointers of a file. */ STATIC int xchk_parent_pptr( @@ -561,14 +778,35 @@ xchk_parent_pptr( if (error) goto out_pp; + /* + * Set up some staging memory for parent pointers that we can't check + * due to locking contention. + */ + error = xfarray_create(sc->mp, "pptr entries", 0, + sizeof(struct xchk_pptr), &pp->pptr_entries); + if (error) + goto out_pp; + + error = xfblob_create(sc->mp, "pptr names", &pp->pptr_names); + if (error) + goto out_entries; + error = xchk_xattr_walk(sc, sc->ip, xchk_parent_scan_attr, pp); if (error == -ECANCELED) { error = 0; - goto out_pp; + goto out_names; } if (error) - goto out_pp; + goto out_names; + error = xchk_parent_finish_slow_pptrs(pp); + if (error) + goto out_names; + +out_names: + xfblob_destroy(pp->pptr_names); +out_entries: + xfarray_destroy(pp->pptr_entries); out_pp: kvfree(pp); return error; diff --git a/fs/xfs/scrub/trace.h b/fs/xfs/scrub/trace.h index ac21759fc3e1..61f18632cb6f 100644 --- a/fs/xfs/scrub/trace.h +++ b/fs/xfs/scrub/trace.h @@ -929,6 +929,39 @@ TRACE_EVENT(xchk_parent_bad_dapos, __get_str(name)) ); +DECLARE_EVENT_CLASS(xchk_pptr_class, + TP_PROTO(struct xfs_inode *ip, const unsigned char *name, + unsigned int namelen, xfs_ino_t parent_ino), + TP_ARGS(ip, name, namelen, parent_ino), + TP_STRUCT__entry( + __field(dev_t, dev) + __field(xfs_ino_t, ino) + __field(unsigned int, namelen) + __dynamic_array(char, name, namelen) + __field(xfs_ino_t, parent_ino) + ), + TP_fast_assign( + __entry->dev = ip->i_mount->m_super->s_dev; + __entry->ino = ip->i_ino; + __entry->namelen = namelen; + memcpy(__get_str(name), name, namelen); + __entry->parent_ino = parent_ino; + ), + TP_printk("dev %d:%d ino 0x%llx name '%.*s' parent_ino 0x%llx", + MAJOR(__entry->dev), MINOR(__entry->dev), + __entry->ino, + __entry->namelen, + __get_str(name), + __entry->parent_ino) +) +#define DEFINE_XCHK_PPTR_CLASS(name) \ +DEFINE_EVENT(xchk_pptr_class, name, \ + TP_PROTO(struct xfs_inode *ip, const unsigned char *name, \ + unsigned int namelen, xfs_ino_t parent_ino), \ + TP_ARGS(ip, name, namelen, parent_ino)) +DEFINE_XCHK_PPTR_CLASS(xchk_parent_defer); +DEFINE_XCHK_PPTR_CLASS(xchk_parent_slowpath); + /* repair tracepoints */ #if IS_ENABLED(CONFIG_XFS_ONLINE_REPAIR)