| From 630a04e79dd41ff746b545d4fc052e0abb836120 Mon Sep 17 00:00:00 2001 |
| From: "Darrick J. Wong" <darrick.wong@oracle.com> |
| Date: Wed, 15 Mar 2017 00:24:25 -0700 |
| Subject: xfs: verify inline directory data forks |
| |
| From: Darrick J. Wong <darrick.wong@oracle.com> |
| |
| commit 630a04e79dd41ff746b545d4fc052e0abb836120 upstream. |
| |
| When we're reading or writing the data fork of an inline directory, |
| check the contents to make sure we're not overflowing buffers or eating |
| garbage data. xfs/348 corrupts an inline symlink into an inline |
| directory, triggering a buffer overflow bug. |
| |
| v2: add more checks consistent with _dir2_sf_check and make the verifier |
| usable from anywhere. |
| |
| Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> |
| Reviewed-by: Brian Foster <bfoster@redhat.com> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| fs/xfs/libxfs/xfs_dir2_priv.h | 2 |
| fs/xfs/libxfs/xfs_dir2_sf.c | 87 +++++++++++++++++++++++++++++++++++++++++ |
| fs/xfs/libxfs/xfs_inode_fork.c | 26 ++++++++++-- |
| fs/xfs/libxfs/xfs_inode_fork.h | 2 |
| fs/xfs/xfs_dir2_readdir.c | 11 ----- |
| fs/xfs/xfs_inode.c | 12 ++++- |
| 6 files changed, 122 insertions(+), 18 deletions(-) |
| |
| --- a/fs/xfs/libxfs/xfs_dir2_priv.h |
| +++ b/fs/xfs/libxfs/xfs_dir2_priv.h |
| @@ -126,6 +126,8 @@ extern int xfs_dir2_sf_create(struct xfs |
| extern int xfs_dir2_sf_lookup(struct xfs_da_args *args); |
| extern int xfs_dir2_sf_removename(struct xfs_da_args *args); |
| extern int xfs_dir2_sf_replace(struct xfs_da_args *args); |
| +extern int xfs_dir2_sf_verify(struct xfs_mount *mp, struct xfs_dir2_sf_hdr *sfp, |
| + int size); |
| |
| /* xfs_dir2_readdir.c */ |
| extern int xfs_readdir(struct xfs_inode *dp, struct dir_context *ctx, |
| --- a/fs/xfs/libxfs/xfs_dir2_sf.c |
| +++ b/fs/xfs/libxfs/xfs_dir2_sf.c |
| @@ -629,6 +629,93 @@ xfs_dir2_sf_check( |
| } |
| #endif /* DEBUG */ |
| |
| +/* Verify the consistency of an inline directory. */ |
| +int |
| +xfs_dir2_sf_verify( |
| + struct xfs_mount *mp, |
| + struct xfs_dir2_sf_hdr *sfp, |
| + int size) |
| +{ |
| + struct xfs_dir2_sf_entry *sfep; |
| + struct xfs_dir2_sf_entry *next_sfep; |
| + char *endp; |
| + const struct xfs_dir_ops *dops; |
| + xfs_ino_t ino; |
| + int i; |
| + int i8count; |
| + int offset; |
| + __uint8_t filetype; |
| + |
| + dops = xfs_dir_get_ops(mp, NULL); |
| + |
| + /* |
| + * Give up if the directory is way too short. |
| + */ |
| + XFS_WANT_CORRUPTED_RETURN(mp, size > |
| + offsetof(struct xfs_dir2_sf_hdr, parent)); |
| + XFS_WANT_CORRUPTED_RETURN(mp, size >= |
| + xfs_dir2_sf_hdr_size(sfp->i8count)); |
| + |
| + endp = (char *)sfp + size; |
| + |
| + /* Check .. entry */ |
| + ino = dops->sf_get_parent_ino(sfp); |
| + i8count = ino > XFS_DIR2_MAX_SHORT_INUM; |
| + XFS_WANT_CORRUPTED_RETURN(mp, !xfs_dir_ino_validate(mp, ino)); |
| + offset = dops->data_first_offset; |
| + |
| + /* Check all reported entries */ |
| + sfep = xfs_dir2_sf_firstentry(sfp); |
| + for (i = 0; i < sfp->count; i++) { |
| + /* |
| + * struct xfs_dir2_sf_entry has a variable length. |
| + * Check the fixed-offset parts of the structure are |
| + * within the data buffer. |
| + */ |
| + XFS_WANT_CORRUPTED_RETURN(mp, |
| + ((char *)sfep + sizeof(*sfep)) < endp); |
| + |
| + /* Don't allow names with known bad length. */ |
| + XFS_WANT_CORRUPTED_RETURN(mp, sfep->namelen > 0); |
| + XFS_WANT_CORRUPTED_RETURN(mp, sfep->namelen < MAXNAMELEN); |
| + |
| + /* |
| + * 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 = dops->sf_nextentry(sfp, sfep); |
| + XFS_WANT_CORRUPTED_RETURN(mp, endp >= (char *)next_sfep); |
| + |
| + /* Check that the offsets always increase. */ |
| + XFS_WANT_CORRUPTED_RETURN(mp, |
| + xfs_dir2_sf_get_offset(sfep) >= offset); |
| + |
| + /* Check the inode number. */ |
| + ino = dops->sf_get_ino(sfp, sfep); |
| + i8count += ino > XFS_DIR2_MAX_SHORT_INUM; |
| + XFS_WANT_CORRUPTED_RETURN(mp, !xfs_dir_ino_validate(mp, ino)); |
| + |
| + /* Check the file type. */ |
| + filetype = dops->sf_get_ftype(sfep); |
| + XFS_WANT_CORRUPTED_RETURN(mp, filetype < XFS_DIR3_FT_MAX); |
| + |
| + offset = xfs_dir2_sf_get_offset(sfep) + |
| + dops->data_entsize(sfep->namelen); |
| + |
| + sfep = next_sfep; |
| + } |
| + XFS_WANT_CORRUPTED_RETURN(mp, i8count == sfp->i8count); |
| + XFS_WANT_CORRUPTED_RETURN(mp, (void *)sfep == (void *)endp); |
| + |
| + /* Make sure this whole thing ought to be in local format. */ |
| + XFS_WANT_CORRUPTED_RETURN(mp, offset + |
| + (sfp->count + 2) * (uint)sizeof(xfs_dir2_leaf_entry_t) + |
| + (uint)sizeof(xfs_dir2_block_tail_t) <= mp->m_dir_geo->blksize); |
| + |
| + return 0; |
| +} |
| + |
| /* |
| * Create a new (shortform) directory. |
| */ |
| --- a/fs/xfs/libxfs/xfs_inode_fork.c |
| +++ b/fs/xfs/libxfs/xfs_inode_fork.c |
| @@ -33,6 +33,8 @@ |
| #include "xfs_trace.h" |
| #include "xfs_attr_sf.h" |
| #include "xfs_da_format.h" |
| +#include "xfs_da_btree.h" |
| +#include "xfs_dir2_priv.h" |
| |
| kmem_zone_t *xfs_ifork_zone; |
| |
| @@ -320,6 +322,7 @@ xfs_iformat_local( |
| int whichfork, |
| int size) |
| { |
| + int error; |
| |
| /* |
| * If the size is unreasonable, then something |
| @@ -336,6 +339,14 @@ xfs_iformat_local( |
| return -EFSCORRUPTED; |
| } |
| |
| + if (S_ISDIR(VFS_I(ip)->i_mode) && whichfork == XFS_DATA_FORK) { |
| + error = xfs_dir2_sf_verify(ip->i_mount, |
| + (struct xfs_dir2_sf_hdr *)XFS_DFORK_DPTR(dip), |
| + size); |
| + if (error) |
| + return error; |
| + } |
| + |
| xfs_init_local_fork(ip, whichfork, XFS_DFORK_PTR(dip, whichfork), size); |
| return 0; |
| } |
| @@ -856,7 +867,7 @@ xfs_iextents_copy( |
| * In these cases, the format always takes precedence, because the |
| * format indicates the current state of the fork. |
| */ |
| -void |
| +int |
| xfs_iflush_fork( |
| xfs_inode_t *ip, |
| xfs_dinode_t *dip, |
| @@ -866,6 +877,7 @@ xfs_iflush_fork( |
| char *cp; |
| xfs_ifork_t *ifp; |
| xfs_mount_t *mp; |
| + int error; |
| static const short brootflag[2] = |
| { XFS_ILOG_DBROOT, XFS_ILOG_ABROOT }; |
| static const short dataflag[2] = |
| @@ -874,7 +886,7 @@ xfs_iflush_fork( |
| { XFS_ILOG_DEXT, XFS_ILOG_AEXT }; |
| |
| if (!iip) |
| - return; |
| + return 0; |
| ifp = XFS_IFORK_PTR(ip, whichfork); |
| /* |
| * This can happen if we gave up in iformat in an error path, |
| @@ -882,12 +894,19 @@ xfs_iflush_fork( |
| */ |
| if (!ifp) { |
| ASSERT(whichfork == XFS_ATTR_FORK); |
| - return; |
| + return 0; |
| } |
| cp = XFS_DFORK_PTR(dip, whichfork); |
| mp = ip->i_mount; |
| switch (XFS_IFORK_FORMAT(ip, whichfork)) { |
| case XFS_DINODE_FMT_LOCAL: |
| + if (S_ISDIR(VFS_I(ip)->i_mode) && whichfork == XFS_DATA_FORK) { |
| + error = xfs_dir2_sf_verify(mp, |
| + (struct xfs_dir2_sf_hdr *)ifp->if_u1.if_data, |
| + ifp->if_bytes); |
| + if (error) |
| + return error; |
| + } |
| if ((iip->ili_fields & dataflag[whichfork]) && |
| (ifp->if_bytes > 0)) { |
| ASSERT(ifp->if_u1.if_data != NULL); |
| @@ -940,6 +959,7 @@ xfs_iflush_fork( |
| ASSERT(0); |
| break; |
| } |
| + return 0; |
| } |
| |
| /* |
| --- a/fs/xfs/libxfs/xfs_inode_fork.h |
| +++ b/fs/xfs/libxfs/xfs_inode_fork.h |
| @@ -140,7 +140,7 @@ typedef struct xfs_ifork { |
| struct xfs_ifork *xfs_iext_state_to_fork(struct xfs_inode *ip, int state); |
| |
| int xfs_iformat_fork(struct xfs_inode *, struct xfs_dinode *); |
| -void xfs_iflush_fork(struct xfs_inode *, struct xfs_dinode *, |
| +int xfs_iflush_fork(struct xfs_inode *, struct xfs_dinode *, |
| struct xfs_inode_log_item *, int); |
| void xfs_idestroy_fork(struct xfs_inode *, int); |
| void xfs_idata_realloc(struct xfs_inode *, int, int); |
| --- a/fs/xfs/xfs_dir2_readdir.c |
| +++ b/fs/xfs/xfs_dir2_readdir.c |
| @@ -71,22 +71,11 @@ xfs_dir2_sf_getdents( |
| struct xfs_da_geometry *geo = args->geo; |
| |
| ASSERT(dp->i_df.if_flags & XFS_IFINLINE); |
| - /* |
| - * Give up if the directory is way too short. |
| - */ |
| - if (dp->i_d.di_size < offsetof(xfs_dir2_sf_hdr_t, parent)) { |
| - ASSERT(XFS_FORCED_SHUTDOWN(dp->i_mount)); |
| - return -EIO; |
| - } |
| - |
| ASSERT(dp->i_df.if_bytes == dp->i_d.di_size); |
| ASSERT(dp->i_df.if_u1.if_data != NULL); |
| |
| sfp = (xfs_dir2_sf_hdr_t *)dp->i_df.if_u1.if_data; |
| |
| - if (dp->i_d.di_size < xfs_dir2_sf_hdr_size(sfp->i8count)) |
| - return -EFSCORRUPTED; |
| - |
| /* |
| * If the block number in the offset is out of range, we're done. |
| */ |
| --- a/fs/xfs/xfs_inode.c |
| +++ b/fs/xfs/xfs_inode.c |
| @@ -3491,6 +3491,7 @@ xfs_iflush_int( |
| struct xfs_inode_log_item *iip = ip->i_itemp; |
| struct xfs_dinode *dip; |
| struct xfs_mount *mp = ip->i_mount; |
| + int error; |
| |
| ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL|XFS_ILOCK_SHARED)); |
| ASSERT(xfs_isiflocked(ip)); |
| @@ -3573,9 +3574,14 @@ xfs_iflush_int( |
| if (ip->i_d.di_flushiter == DI_MAX_FLUSH) |
| ip->i_d.di_flushiter = 0; |
| |
| - xfs_iflush_fork(ip, dip, iip, XFS_DATA_FORK); |
| - if (XFS_IFORK_Q(ip)) |
| - xfs_iflush_fork(ip, dip, iip, XFS_ATTR_FORK); |
| + error = xfs_iflush_fork(ip, dip, iip, XFS_DATA_FORK); |
| + if (error) |
| + return error; |
| + if (XFS_IFORK_Q(ip)) { |
| + error = xfs_iflush_fork(ip, dip, iip, XFS_ATTR_FORK); |
| + if (error) |
| + return error; |
| + } |
| xfs_inobp_check(mp, bp); |
| |
| /* |