xfs: rework the inline directory verifiers
Source kernel commit: 78420281a9d74014af7616958806c3aba056319e
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: Brian Foster <bfoster@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Eric Sandeen <sandeen@redhat.com>
Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
diff --git a/libxfs/util.c b/libxfs/util.c
index cd802e8..dcfca39 100644
--- a/libxfs/util.c
+++ b/libxfs/util.c
@@ -36,6 +36,9 @@
#include "xfs_ialloc.h"
#include "xfs_alloc.h"
#include "xfs_bit.h"
+#include "xfs_da_format.h"
+#include "xfs_da_btree.h"
+#include "xfs_dir2_priv.h"
/*
* Calculate the worst case log unit reservation for a given superblock
@@ -433,6 +436,12 @@
if (ip->i_d.di_version == 3)
VFS_I(ip)->i_version++;
+ /* 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))
+ return -EFSCORRUPTED;
+
/*
* Copy the dirty parts of the inode into the on-disk
* inode. We always copy out the core of the inode,
diff --git a/libxfs/xfs_dir2_priv.h b/libxfs/xfs_dir2_priv.h
index eb00bc1..39f8604 100644
--- a/libxfs/xfs_dir2_priv.h
+++ b/libxfs/xfs_dir2_priv.h
@@ -125,8 +125,7 @@
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);
+extern int xfs_dir2_sf_verify(struct xfs_inode *ip);
/* xfs_dir2_readdir.c */
extern int xfs_readdir(struct xfs_inode *dp, struct dir_context *ctx,
diff --git a/libxfs/xfs_dir2_sf.c b/libxfs/xfs_dir2_sf.c
index 712afa5..195f816 100644
--- a/libxfs/xfs_dir2_sf.c
+++ b/libxfs/xfs_dir2_sf.c
@@ -630,36 +630,49 @@
/* 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_inode *ip)
{
+ struct xfs_mount *mp = ip->i_mount;
+ struct xfs_dir2_sf_hdr *sfp;
struct xfs_dir2_sf_entry *sfep;
struct xfs_dir2_sf_entry *next_sfep;
char *endp;
const struct xfs_dir_ops *dops;
+ struct xfs_ifork *ifp;
xfs_ino_t ino;
int i;
int i8count;
int offset;
+ int size;
+ int error;
__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);
+ 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.
*/
- 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));
+ if (size <= offsetof(struct xfs_dir2_sf_hdr, parent) ||
+ size < xfs_dir2_sf_hdr_size(sfp->i8count))
+ return -EFSCORRUPTED;
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));
+ error = xfs_dir_ino_validate(mp, ino);
+ if (error)
+ return error;
offset = dops->data_first_offset;
/* Check all reported entries */
@@ -670,12 +683,12 @@
* Check the fixed-offset parts of the structure are
* within the data buffer.
*/
- XFS_WANT_CORRUPTED_RETURN(mp,
- ((char *)sfep + sizeof(*sfep)) < endp);
+ if (((char *)sfep + sizeof(*sfep)) >= endp)
+ return -EFSCORRUPTED;
/* Don't allow names with known bad length. */
- XFS_WANT_CORRUPTED_RETURN(mp, sfep->namelen > 0);
- XFS_WANT_CORRUPTED_RETURN(mp, sfep->namelen < MAXNAMELEN);
+ if (sfep->namelen == 0)
+ return -EFSCORRUPTED;
/*
* Check that the variable-length part of the structure is
@@ -683,33 +696,39 @@
* name component, so nextentry is an acceptable test.
*/
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. */
- XFS_WANT_CORRUPTED_RETURN(mp,
- xfs_dir2_sf_get_offset(sfep) >= offset);
+ if (xfs_dir2_sf_get_offset(sfep) < offset)
+ return -EFSCORRUPTED;
/* 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));
+ error = xfs_dir_ino_validate(mp, ino);
+ if (error)
+ return error;
/* Check the file type. */
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) +
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);
+ if (i8count != sfp->i8count)
+ return -EFSCORRUPTED;
+ if ((void *)sfep != (void *)endp)
+ return -EFSCORRUPTED;
/* 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);
+ if (offset + (sfp->count + 2) * (uint)sizeof(xfs_dir2_leaf_entry_t) +
+ (uint)sizeof(xfs_dir2_block_tail_t) > mp->m_dir_geo->blksize)
+ return -EFSCORRUPTED;
return 0;
}
diff --git a/libxfs/xfs_inode_fork.c b/libxfs/xfs_inode_fork.c
index cc7f36b..814625c 100644
--- a/libxfs/xfs_inode_fork.c
+++ b/libxfs/xfs_inode_fork.c
@@ -209,6 +209,16 @@
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);
@@ -319,8 +329,6 @@
int whichfork,
int size)
{
- int error;
-
/*
* If the size is unreasonable, then something
* is wrong and we just bail out rather than crash in
@@ -336,14 +344,6 @@
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;
}
@@ -864,7 +864,7 @@
* In these cases, the format always takes precedence, because the
* format indicates the current state of the fork.
*/
-int
+void
xfs_iflush_fork(
xfs_inode_t *ip,
xfs_dinode_t *dip,
@@ -874,7 +874,6 @@
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] =
@@ -883,7 +882,7 @@
{ XFS_ILOG_DEXT, XFS_ILOG_AEXT };
if (!iip)
- return 0;
+ return;
ifp = XFS_IFORK_PTR(ip, whichfork);
/*
* This can happen if we gave up in iformat in an error path,
@@ -891,19 +890,12 @@
*/
if (!ifp) {
ASSERT(whichfork == XFS_ATTR_FORK);
- return 0;
+ return;
}
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);
@@ -956,7 +948,6 @@
ASSERT(0);
break;
}
- return 0;
}
/*
diff --git a/libxfs/xfs_inode_fork.h b/libxfs/xfs_inode_fork.h
index 132dc59..7fb8365 100644
--- a/libxfs/xfs_inode_fork.h
+++ b/libxfs/xfs_inode_fork.h
@@ -140,7 +140,7 @@
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_iflush_fork(struct xfs_inode *, struct xfs_dinode *,
+void 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);