From patchwork Wed Jul 19 16:03:18 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Darrick J. Wong" X-Patchwork-Id: 9852749 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 96712602BD for ; Wed, 19 Jul 2017 16:03:50 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 865AD201F5 for ; Wed, 19 Jul 2017 16:03:50 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 7AC63237F1; Wed, 19 Jul 2017 16:03:50 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 593B8201F5 for ; Wed, 19 Jul 2017 16:03:49 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754795AbdGSQDr (ORCPT ); Wed, 19 Jul 2017 12:03:47 -0400 Received: from aserp1040.oracle.com ([141.146.126.69]:38492 "EHLO aserp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755259AbdGSQDW (ORCPT ); Wed, 19 Jul 2017 12:03:22 -0400 Received: from aserv0021.oracle.com (aserv0021.oracle.com [141.146.126.233]) by aserp1040.oracle.com (Sentrion-MTA-4.3.2/Sentrion-MTA-4.3.2) with ESMTP id v6JG3KL1004367 (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 19 Jul 2017 16:03:20 GMT Received: from userv0121.oracle.com (userv0121.oracle.com [156.151.31.72]) by aserv0021.oracle.com (8.13.8/8.14.4) with ESMTP id v6JG3J9J009919 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 19 Jul 2017 16:03:20 GMT Received: from abhmp0010.oracle.com (abhmp0010.oracle.com [141.146.116.16]) by userv0121.oracle.com (8.14.4/8.13.8) with ESMTP id v6JG3Jnk028230; Wed, 19 Jul 2017 16:03:19 GMT Received: from localhost (/73.25.142.12) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Wed, 19 Jul 2017 09:03:19 -0700 Date: Wed, 19 Jul 2017 09:03:18 -0700 From: "Darrick J. Wong" To: xfs Cc: Dave Chinner Subject: [RFC PATCH] xfs: consolidate local format inode fork verifiers Message-ID: <20170719160318.GP4224@magnolia> MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.5.24 (2015-08-30) X-Source-IP: aserv0021.oracle.com [141.146.126.233] Sender: linux-xfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Create a centralized function to verify local format inode forks, then migrate the existing short format directory verifier to use this framework and add verifiers for the other two things we support (attrs and symlinks). Obviously this will get broken up into smaller pieces (one patch to refactor/move the existing verifier calls, another one to add the two new verifiers), but what do people think? Signed-off-by: Darrick J. Wong --- fs/xfs/libxfs/xfs_attr_leaf.c | 70 ++++++++++++++++++++++++++++++++++++ fs/xfs/libxfs/xfs_attr_leaf.h | 1 + fs/xfs/libxfs/xfs_inode_fork.c | 10 ----- fs/xfs/libxfs/xfs_shared.h | 1 + fs/xfs/libxfs/xfs_symlink_remote.c | 29 +++++++++++++++ fs/xfs/xfs_icache.c | 46 ++++++++++++++++++++++++ fs/xfs/xfs_icache.h | 1 + fs/xfs/xfs_inode.c | 6 +-- 8 files changed, 150 insertions(+), 14 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c index c6c15e5..43b881e 100644 --- a/fs/xfs/libxfs/xfs_attr_leaf.c +++ b/fs/xfs/libxfs/xfs_attr_leaf.c @@ -871,6 +871,76 @@ xfs_attr_shortform_allfit( return xfs_attr_shortform_bytesfit(dp, bytes); } +/* Verify the consistency of an inline attribute fork. */ +int +xfs_attr_shortform_verify( + struct xfs_inode *ip) +{ + struct xfs_attr_shortform *sfp; + struct xfs_attr_sf_entry *sfep; + struct xfs_attr_sf_entry *next_sfep; + char *endp; + struct xfs_ifork *ifp; + int i; + int size; + + ASSERT(ip->i_d.di_aformat == XFS_DINODE_FMT_LOCAL); + ifp = XFS_IFORK_PTR(ip, XFS_ATTR_FORK); + sfp = (struct xfs_attr_shortform *)ifp->if_u1.if_data; + size = ifp->if_bytes; + + /* + * Give up if the attribute is way too short. + */ + if (size < sizeof (struct xfs_attr_sf_hdr)) + return -EFSCORRUPTED; + + endp = (char *)sfp + size; + + /* Check all reported entries */ + sfep = &sfp->list[0]; + for (i = 0; i < sfp->hdr.count; i++) { + /* + * struct xfs_attr_sf_entry has a variable length. + * Check the fixed-offset parts of the structure are + * within the data buffer. + */ + if (((char *)sfep + sizeof(*sfep)) >= endp) + return -EFSCORRUPTED; + + /* Don't allow names with known bad length. */ + if (sfep->namelen == 0) + return -EFSCORRUPTED; + + /* + * Check that the variable-length part of the structure is + * within the data buffer. The next entry starts after the + * name component, so nextentry is an acceptable test. + */ + next_sfep = XFS_ATTR_SF_NEXTENTRY(sfep); + if (endp < (char *)next_sfep) + return -EFSCORRUPTED; + + /* + * Check for unknown flags. Short form doesn't support + * the incomplete or local bits. + */ + if (sfep->flags & ~(XFS_ATTR_ROOT | XFS_ATTR_SECURE)) + return -EFSCORRUPTED; + + /* Check for invalid namespace combinations. */ + if ((sfep->flags & XFS_ATTR_ROOT) && + (sfep->flags & XFS_ATTR_SECURE)) + return -EFSCORRUPTED; + + sfep = next_sfep; + } + if ((void *)sfep != (void *)endp) + return -EFSCORRUPTED; + + return 0; +} + /* * Convert a leaf attribute list to shortform attribute list */ diff --git a/fs/xfs/libxfs/xfs_attr_leaf.h b/fs/xfs/libxfs/xfs_attr_leaf.h index f7dda0c..ec7291d 100644 --- a/fs/xfs/libxfs/xfs_attr_leaf.h +++ b/fs/xfs/libxfs/xfs_attr_leaf.h @@ -52,6 +52,7 @@ int xfs_attr_shortform_to_leaf(struct xfs_da_args *args); int xfs_attr_shortform_remove(struct xfs_da_args *args); int xfs_attr_shortform_allfit(struct xfs_buf *bp, struct xfs_inode *dp); int xfs_attr_shortform_bytesfit(struct xfs_inode *dp, int bytes); +int xfs_attr_shortform_verify(struct xfs_inode *ip); void xfs_attr_fork_remove(struct xfs_inode *ip, struct xfs_trans *tp); /* diff --git a/fs/xfs/libxfs/xfs_inode_fork.c b/fs/xfs/libxfs/xfs_inode_fork.c index 0e80f34..5484211 100644 --- a/fs/xfs/libxfs/xfs_inode_fork.c +++ b/fs/xfs/libxfs/xfs_inode_fork.c @@ -183,16 +183,6 @@ xfs_iformat_fork( if (error) return error; - /* Check inline dir contents. */ - if (S_ISDIR(VFS_I(ip)->i_mode) && - dip->di_format == XFS_DINODE_FMT_LOCAL) { - error = xfs_dir2_sf_verify(ip); - if (error) { - xfs_idestroy_fork(ip, XFS_DATA_FORK); - return error; - } - } - if (xfs_is_reflink_inode(ip)) { ASSERT(ip->i_cowfp == NULL); xfs_ifork_init_cow(ip); diff --git a/fs/xfs/libxfs/xfs_shared.h b/fs/xfs/libxfs/xfs_shared.h index c6f4eb4..7e35a16 100644 --- a/fs/xfs/libxfs/xfs_shared.h +++ b/fs/xfs/libxfs/xfs_shared.h @@ -143,5 +143,6 @@ bool xfs_symlink_hdr_ok(xfs_ino_t ino, uint32_t offset, uint32_t size, struct xfs_buf *bp); void xfs_symlink_local_to_remote(struct xfs_trans *tp, struct xfs_buf *bp, struct xfs_inode *ip, struct xfs_ifork *ifp); +int xfs_symlink_sf_verify(struct xfs_inode *ip); #endif /* __XFS_SHARED_H__ */ diff --git a/fs/xfs/libxfs/xfs_symlink_remote.c b/fs/xfs/libxfs/xfs_symlink_remote.c index c484877..96e2957 100644 --- a/fs/xfs/libxfs/xfs_symlink_remote.c +++ b/fs/xfs/libxfs/xfs_symlink_remote.c @@ -207,3 +207,32 @@ xfs_symlink_local_to_remote( xfs_trans_log_buf(tp, bp, 0, sizeof(struct xfs_dsymlink_hdr) + ifp->if_bytes - 1); } + +/* Verify the consistency of an inline symlink. */ +int +xfs_symlink_sf_verify( + struct xfs_inode *ip) +{ + char *sfp; + char *endp; + struct xfs_ifork *ifp; + int size; + + ASSERT(ip->i_d.di_format == XFS_DINODE_FMT_LOCAL); + ifp = XFS_IFORK_PTR(ip, XFS_DATA_FORK); + sfp = (char *)ifp->if_u1.if_data; + size = ifp->if_bytes; + endp = sfp + size; + + /* No negative sizes or overly long symlink targets. */ + if (size < 0 || size > XFS_SYMLINK_MAXLEN) + return -EFSCORRUPTED; + + /* No NULLs in the target either. */ + for (; sfp < endp; sfp++) { + if (*sfp == 0) + return -EFSCORRUPTED; + } + + return 0; +} diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c index 0a9e698..fd1d430 100644 --- a/fs/xfs/xfs_icache.c +++ b/fs/xfs/xfs_icache.c @@ -34,6 +34,11 @@ #include "xfs_dquot_item.h" #include "xfs_dquot.h" #include "xfs_reflink.h" +#include "xfs_da_format.h" +#include "xfs_da_btree.h" +#include "xfs_dir2_priv.h" +#include "xfs_attr_leaf.h" +#include "xfs_shared.h" #include #include @@ -449,6 +454,43 @@ xfs_iget_cache_hit( return error; } +/* Check inline fork contents. */ +int +xfs_iget_verify_forks( + struct xfs_inode *ip) +{ + int error; + + /* Check the attribute fork if there is one. */ + if (XFS_IFORK_PTR(ip, XFS_ATTR_FORK) && + ip->i_d.di_aformat == XFS_DINODE_FMT_LOCAL) { + error = xfs_attr_shortform_verify(ip); + if (error) { + xfs_alert(ip->i_mount, + "%s: bad inode %Lu inline attr fork", + __func__, ip->i_ino); + return error; + } + } + + /* Non-local data fork, jump out. */ + if (ip->i_d.di_format != XFS_DINODE_FMT_LOCAL) + return 0; + + /* Check the inline data fork if there is one. */ + if (S_ISDIR(VFS_I(ip)->i_mode)) + error = xfs_dir2_sf_verify(ip); + else if (S_ISLNK(VFS_I(ip)->i_mode)) + error = xfs_symlink_sf_verify(ip); + else + error = -EFSCORRUPTED; + + if (error) + xfs_alert(ip->i_mount, "%s: bad inode %Lu inline data fork", + __func__, ip->i_ino); + return error; +} + static int xfs_iget_cache_miss( @@ -473,6 +515,10 @@ xfs_iget_cache_miss( if (error) goto out_destroy; + error = xfs_iget_verify_forks(ip); + if (error) + goto out_destroy; + trace_xfs_iget_miss(ip); if ((VFS_I(ip)->i_mode == 0) && !(flags & XFS_IGET_CREATE)) { diff --git a/fs/xfs/xfs_icache.h b/fs/xfs/xfs_icache.h index bff4d85..abf1cff 100644 --- a/fs/xfs/xfs_icache.h +++ b/fs/xfs/xfs_icache.h @@ -129,5 +129,6 @@ xfs_fs_eofblocks_from_user( int xfs_icache_inode_is_allocated(struct xfs_mount *mp, struct xfs_trans *tp, xfs_ino_t ino, bool *inuse); +int xfs_iget_verify_forks(struct xfs_inode *ip); #endif diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c index ceef77c..7ca8336 100644 --- a/fs/xfs/xfs_inode.c +++ b/fs/xfs/xfs_inode.c @@ -3547,10 +3547,8 @@ xfs_iflush_int( if (ip->i_d.di_version < 3) ip->i_d.di_flushiter++; - /* Check the inline directory data. */ - if (S_ISDIR(VFS_I(ip)->i_mode) && - ip->i_d.di_format == XFS_DINODE_FMT_LOCAL && - xfs_dir2_sf_verify(ip)) + /* Check the inline fork data. */ + if (xfs_iget_verify_forks(ip)) goto corrupt_out; /*