Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit ca659e08 authored by Darrick J. Wong's avatar Darrick J. Wong Committed by Greg Kroah-Hartman
Browse files

xfs: rework the inline directory verifiers



commit 78420281a9d74014af7616958806c3aba056319e upstream.

The inline directory verifiers should be called on the inode fork data,
which means after iformat_local on the read side, and prior to
ifork_flush on the write side.  This makes the fork verifier more
consistent with the way buffer verifiers work -- i.e. they will operate
on the memory buffer that the code will be reading and writing directly.

Furthermore, revise the verifier function to return -EFSCORRUPTED so
that we don't flood the logs with corruption messages and assert
notices.  This has been a particular problem with xfs/348, which
triggers the XFS_WANT_CORRUPTED_RETURN assertions, which halts the
kernel when CONFIG_XFS_DEBUG=y.  Disk corruption isn't supposed to do
that, at least not in a verifier.

Reviewed-by: default avatarBrian Foster <bfoster@redhat.com>
Reviewed-by: default avatarChristoph Hellwig <hch@lst.de>
Signed-off-by: default avatarDarrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 815414e7
Loading
Loading
Loading
Loading
+1 −2
Original line number Original line Diff line number Diff line
@@ -126,8 +126,7 @@ extern int xfs_dir2_sf_create(struct xfs_da_args *args, xfs_ino_t pino);
extern int xfs_dir2_sf_lookup(struct xfs_da_args *args);
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_removename(struct xfs_da_args *args);
extern int xfs_dir2_sf_replace(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,
extern int xfs_dir2_sf_verify(struct xfs_inode *ip);
		int size);


/* xfs_dir2_readdir.c */
/* xfs_dir2_readdir.c */
extern int xfs_readdir(struct xfs_inode *dp, struct dir_context *ctx,
extern int xfs_readdir(struct xfs_inode *dp, struct dir_context *ctx,
+41 −22
Original line number Original line Diff line number Diff line
@@ -632,36 +632,49 @@ xfs_dir2_sf_check(
/* Verify the consistency of an inline directory. */
/* Verify the consistency of an inline directory. */
int
int
xfs_dir2_sf_verify(
xfs_dir2_sf_verify(
	struct xfs_mount		*mp,
	struct xfs_inode		*ip)
	struct xfs_dir2_sf_hdr		*sfp,
	int				size)
{
{
	struct xfs_mount		*mp = ip->i_mount;
	struct xfs_dir2_sf_hdr		*sfp;
	struct xfs_dir2_sf_entry	*sfep;
	struct xfs_dir2_sf_entry	*sfep;
	struct xfs_dir2_sf_entry	*next_sfep;
	struct xfs_dir2_sf_entry	*next_sfep;
	char				*endp;
	char				*endp;
	const struct xfs_dir_ops	*dops;
	const struct xfs_dir_ops	*dops;
	struct xfs_ifork		*ifp;
	xfs_ino_t			ino;
	xfs_ino_t			ino;
	int				i;
	int				i;
	int				i8count;
	int				i8count;
	int				offset;
	int				offset;
	int				size;
	int				error;
	__uint8_t			filetype;
	__uint8_t			filetype;


	ASSERT(ip->i_d.di_format == XFS_DINODE_FMT_LOCAL);
	/*
	 * xfs_iread calls us before xfs_setup_inode sets up ip->d_ops,
	 * so we can only trust the mountpoint to have the right pointer.
	 */
	dops = xfs_dir_get_ops(mp, NULL);
	dops = xfs_dir_get_ops(mp, NULL);


	ifp = XFS_IFORK_PTR(ip, XFS_DATA_FORK);
	sfp = (struct xfs_dir2_sf_hdr *)ifp->if_u1.if_data;
	size = ifp->if_bytes;

	/*
	/*
	 * Give up if the directory is way too short.
	 * Give up if the directory is way too short.
	 */
	 */
	XFS_WANT_CORRUPTED_RETURN(mp, size >
	if (size <= offsetof(struct xfs_dir2_sf_hdr, parent) ||
			offsetof(struct xfs_dir2_sf_hdr, parent));
	    size < xfs_dir2_sf_hdr_size(sfp->i8count))
	XFS_WANT_CORRUPTED_RETURN(mp, size >=
		return -EFSCORRUPTED;
			xfs_dir2_sf_hdr_size(sfp->i8count));


	endp = (char *)sfp + size;
	endp = (char *)sfp + size;


	/* Check .. entry */
	/* Check .. entry */
	ino = dops->sf_get_parent_ino(sfp);
	ino = dops->sf_get_parent_ino(sfp);
	i8count = ino > XFS_DIR2_MAX_SHORT_INUM;
	i8count = ino > XFS_DIR2_MAX_SHORT_INUM;
	XFS_WANT_CORRUPTED_RETURN(mp, !xfs_dir_ino_validate(mp, ino));
	error = xfs_dir_ino_validate(mp, ino);
	if (error)
		return error;
	offset = dops->data_first_offset;
	offset = dops->data_first_offset;


	/* Check all reported entries */
	/* Check all reported entries */
@@ -672,12 +685,12 @@ xfs_dir2_sf_verify(
		 * Check the fixed-offset parts of the structure are
		 * Check the fixed-offset parts of the structure are
		 * within the data buffer.
		 * within the data buffer.
		 */
		 */
		XFS_WANT_CORRUPTED_RETURN(mp,
		if (((char *)sfep + sizeof(*sfep)) >= endp)
				((char *)sfep + sizeof(*sfep)) < endp);
			return -EFSCORRUPTED;


		/* Don't allow names with known bad length. */
		/* Don't allow names with known bad length. */
		XFS_WANT_CORRUPTED_RETURN(mp, sfep->namelen > 0);
		if (sfep->namelen == 0)
		XFS_WANT_CORRUPTED_RETURN(mp, sfep->namelen < MAXNAMELEN);
			return -EFSCORRUPTED;


		/*
		/*
		 * Check that the variable-length part of the structure is
		 * Check that the variable-length part of the structure is
@@ -685,33 +698,39 @@ xfs_dir2_sf_verify(
		 * name component, so nextentry is an acceptable test.
		 * name component, so nextentry is an acceptable test.
		 */
		 */
		next_sfep = dops->sf_nextentry(sfp, sfep);
		next_sfep = dops->sf_nextentry(sfp, sfep);
		XFS_WANT_CORRUPTED_RETURN(mp, endp >= (char *)next_sfep);
		if (endp < (char *)next_sfep)
			return -EFSCORRUPTED;


		/* Check that the offsets always increase. */
		/* Check that the offsets always increase. */
		XFS_WANT_CORRUPTED_RETURN(mp,
		if (xfs_dir2_sf_get_offset(sfep) < offset)
				xfs_dir2_sf_get_offset(sfep) >= offset);
			return -EFSCORRUPTED;


		/* Check the inode number. */
		/* Check the inode number. */
		ino = dops->sf_get_ino(sfp, sfep);
		ino = dops->sf_get_ino(sfp, sfep);
		i8count += ino > XFS_DIR2_MAX_SHORT_INUM;
		i8count += ino > XFS_DIR2_MAX_SHORT_INUM;
		XFS_WANT_CORRUPTED_RETURN(mp, !xfs_dir_ino_validate(mp, ino));
		error = xfs_dir_ino_validate(mp, ino);
		if (error)
			return error;


		/* Check the file type. */
		/* Check the file type. */
		filetype = dops->sf_get_ftype(sfep);
		filetype = dops->sf_get_ftype(sfep);
		XFS_WANT_CORRUPTED_RETURN(mp, filetype < XFS_DIR3_FT_MAX);
		if (filetype >= XFS_DIR3_FT_MAX)
			return -EFSCORRUPTED;


		offset = xfs_dir2_sf_get_offset(sfep) +
		offset = xfs_dir2_sf_get_offset(sfep) +
				dops->data_entsize(sfep->namelen);
				dops->data_entsize(sfep->namelen);


		sfep = next_sfep;
		sfep = next_sfep;
	}
	}
	XFS_WANT_CORRUPTED_RETURN(mp, i8count == sfp->i8count);
	if (i8count != sfp->i8count)
	XFS_WANT_CORRUPTED_RETURN(mp, (void *)sfep == (void *)endp);
		return -EFSCORRUPTED;
	if ((void *)sfep != (void *)endp)
		return -EFSCORRUPTED;


	/* Make sure this whole thing ought to be in local format. */
	/* Make sure this whole thing ought to be in local format. */
	XFS_WANT_CORRUPTED_RETURN(mp, offset +
	if (offset + (sfp->count + 2) * (uint)sizeof(xfs_dir2_leaf_entry_t) +
	       (sfp->count + 2) * (uint)sizeof(xfs_dir2_leaf_entry_t) +
	    (uint)sizeof(xfs_dir2_block_tail_t) > mp->m_dir_geo->blksize)
	       (uint)sizeof(xfs_dir2_block_tail_t) <= mp->m_dir_geo->blksize);
		return -EFSCORRUPTED;


	return 0;
	return 0;
}
}
+13 −22
Original line number Original line Diff line number Diff line
@@ -212,6 +212,16 @@ xfs_iformat_fork(
	if (error)
	if (error)
		return 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)) {
	if (xfs_is_reflink_inode(ip)) {
		ASSERT(ip->i_cowfp == NULL);
		ASSERT(ip->i_cowfp == NULL);
		xfs_ifork_init_cow(ip);
		xfs_ifork_init_cow(ip);
@@ -322,8 +332,6 @@ xfs_iformat_local(
	int		whichfork,
	int		whichfork,
	int		size)
	int		size)
{
{
	int		error;

	/*
	/*
	 * If the size is unreasonable, then something
	 * If the size is unreasonable, then something
	 * is wrong and we just bail out rather than crash in
	 * is wrong and we just bail out rather than crash in
@@ -339,14 +347,6 @@ xfs_iformat_local(
		return -EFSCORRUPTED;
		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);
	xfs_init_local_fork(ip, whichfork, XFS_DFORK_PTR(dip, whichfork), size);
	return 0;
	return 0;
}
}
@@ -867,7 +867,7 @@ xfs_iextents_copy(
 * In these cases, the format always takes precedence, because the
 * In these cases, the format always takes precedence, because the
 * format indicates the current state of the fork.
 * format indicates the current state of the fork.
 */
 */
int
void
xfs_iflush_fork(
xfs_iflush_fork(
	xfs_inode_t		*ip,
	xfs_inode_t		*ip,
	xfs_dinode_t		*dip,
	xfs_dinode_t		*dip,
@@ -877,7 +877,6 @@ xfs_iflush_fork(
	char			*cp;
	char			*cp;
	xfs_ifork_t		*ifp;
	xfs_ifork_t		*ifp;
	xfs_mount_t		*mp;
	xfs_mount_t		*mp;
	int			error;
	static const short	brootflag[2] =
	static const short	brootflag[2] =
		{ XFS_ILOG_DBROOT, XFS_ILOG_ABROOT };
		{ XFS_ILOG_DBROOT, XFS_ILOG_ABROOT };
	static const short	dataflag[2] =
	static const short	dataflag[2] =
@@ -886,7 +885,7 @@ xfs_iflush_fork(
		{ XFS_ILOG_DEXT, XFS_ILOG_AEXT };
		{ XFS_ILOG_DEXT, XFS_ILOG_AEXT };


	if (!iip)
	if (!iip)
		return 0;
		return;
	ifp = XFS_IFORK_PTR(ip, whichfork);
	ifp = XFS_IFORK_PTR(ip, whichfork);
	/*
	/*
	 * This can happen if we gave up in iformat in an error path,
	 * This can happen if we gave up in iformat in an error path,
@@ -894,19 +893,12 @@ xfs_iflush_fork(
	 */
	 */
	if (!ifp) {
	if (!ifp) {
		ASSERT(whichfork == XFS_ATTR_FORK);
		ASSERT(whichfork == XFS_ATTR_FORK);
		return 0;
		return;
	}
	}
	cp = XFS_DFORK_PTR(dip, whichfork);
	cp = XFS_DFORK_PTR(dip, whichfork);
	mp = ip->i_mount;
	mp = ip->i_mount;
	switch (XFS_IFORK_FORMAT(ip, whichfork)) {
	switch (XFS_IFORK_FORMAT(ip, whichfork)) {
	case XFS_DINODE_FMT_LOCAL:
	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]) &&
		if ((iip->ili_fields & dataflag[whichfork]) &&
		    (ifp->if_bytes > 0)) {
		    (ifp->if_bytes > 0)) {
			ASSERT(ifp->if_u1.if_data != NULL);
			ASSERT(ifp->if_u1.if_data != NULL);
@@ -959,7 +951,6 @@ xfs_iflush_fork(
		ASSERT(0);
		ASSERT(0);
		break;
		break;
	}
	}
	return 0;
}
}


/*
/*
+1 −1
Original line number Original line Diff line number Diff line
@@ -140,7 +140,7 @@ typedef struct xfs_ifork {
struct xfs_ifork *xfs_iext_state_to_fork(struct xfs_inode *ip, int state);
struct xfs_ifork *xfs_iext_state_to_fork(struct xfs_inode *ip, int state);


int		xfs_iformat_fork(struct xfs_inode *, struct xfs_dinode *);
int		xfs_iformat_fork(struct xfs_inode *, struct xfs_dinode *);
int		xfs_iflush_fork(struct xfs_inode *, struct xfs_dinode *,
void		xfs_iflush_fork(struct xfs_inode *, struct xfs_dinode *,
				struct xfs_inode_log_item *, int);
				struct xfs_inode_log_item *, int);
void		xfs_idestroy_fork(struct xfs_inode *, int);
void		xfs_idestroy_fork(struct xfs_inode *, int);
void		xfs_idata_realloc(struct xfs_inode *, int, int);
void		xfs_idata_realloc(struct xfs_inode *, int, int);
+10 −9
Original line number Original line Diff line number Diff line
@@ -50,6 +50,7 @@
#include "xfs_log.h"
#include "xfs_log.h"
#include "xfs_bmap_btree.h"
#include "xfs_bmap_btree.h"
#include "xfs_reflink.h"
#include "xfs_reflink.h"
#include "xfs_dir2_priv.h"


kmem_zone_t *xfs_inode_zone;
kmem_zone_t *xfs_inode_zone;


@@ -3491,7 +3492,6 @@ xfs_iflush_int(
	struct xfs_inode_log_item *iip = ip->i_itemp;
	struct xfs_inode_log_item *iip = ip->i_itemp;
	struct xfs_dinode	*dip;
	struct xfs_dinode	*dip;
	struct xfs_mount	*mp = ip->i_mount;
	struct xfs_mount	*mp = ip->i_mount;
	int			error;


	ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL|XFS_ILOCK_SHARED));
	ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL|XFS_ILOCK_SHARED));
	ASSERT(xfs_isiflocked(ip));
	ASSERT(xfs_isiflocked(ip));
@@ -3563,6 +3563,12 @@ xfs_iflush_int(
	if (ip->i_d.di_version < 3)
	if (ip->i_d.di_version < 3)
		ip->i_d.di_flushiter++;
		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))
		goto corrupt_out;

	/*
	/*
	 * Copy the dirty parts of the inode into the on-disk inode.  We always
	 * Copy the dirty parts of the inode into the on-disk inode.  We always
	 * copy out the core of the inode, because if the inode is dirty at all
	 * copy out the core of the inode, because if the inode is dirty at all
@@ -3574,14 +3580,9 @@ xfs_iflush_int(
	if (ip->i_d.di_flushiter == DI_MAX_FLUSH)
	if (ip->i_d.di_flushiter == DI_MAX_FLUSH)
		ip->i_d.di_flushiter = 0;
		ip->i_d.di_flushiter = 0;


	error = xfs_iflush_fork(ip, dip, iip, XFS_DATA_FORK);
	xfs_iflush_fork(ip, dip, iip, XFS_DATA_FORK);
	if (error)
	if (XFS_IFORK_Q(ip))
		return error;
		xfs_iflush_fork(ip, dip, iip, XFS_ATTR_FORK);
	if (XFS_IFORK_Q(ip)) {
		error = xfs_iflush_fork(ip, dip, iip, XFS_ATTR_FORK);
		if (error)
			return error;
	}
	xfs_inobp_check(mp, bp);
	xfs_inobp_check(mp, bp);


	/*
	/*