From: <gregkh@linuxfoundation.org>
To: darrick.wong@oracle.com, bfoster@redhat.com,
gregkh@linuxfoundation.org, hch@lst.de
Cc: <stable@vger.kernel.org>, <stable-commits@vger.kernel.org>
Subject: Patch "xfs: rework the inline directory verifiers" has been added to the 4.9-stable tree
Date: Mon, 05 Jun 2017 16:09:02 +0200 [thread overview]
Message-ID: <149667174215513@kroah.com> (raw)
This is a note to let you know that I've just added the patch titled
xfs: rework the inline directory verifiers
to the 4.9-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
xfs-rework-the-inline-directory-verifiers.patch
and it can be found in the queue-4.9 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@vger.kernel.org> know about it.
>From 78420281a9d74014af7616958806c3aba056319e Mon Sep 17 00:00:00 2001
From: "Darrick J. Wong" <darrick.wong@oracle.com>
Date: Mon, 3 Apr 2017 12:22:20 -0700
Subject: xfs: rework the inline directory verifiers
From: Darrick J. Wong <darrick.wong@oracle.com>
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: Brian Foster <bfoster@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
fs/xfs/libxfs/xfs_dir2_priv.h | 3 -
fs/xfs/libxfs/xfs_dir2_sf.c | 63 ++++++++++++++++++++++++++---------------
fs/xfs/libxfs/xfs_inode_fork.c | 35 ++++++++--------------
fs/xfs/libxfs/xfs_inode_fork.h | 2 -
fs/xfs/xfs_inode.c | 19 ++++++------
5 files changed, 66 insertions(+), 56 deletions(-)
--- a/fs/xfs/libxfs/xfs_dir2_priv.h
+++ b/fs/xfs/libxfs/xfs_dir2_priv.h
@@ -126,8 +126,7 @@ 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);
+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,
--- a/fs/xfs/libxfs/xfs_dir2_sf.c
+++ b/fs/xfs/libxfs/xfs_dir2_sf.c
@@ -632,36 +632,49 @@ xfs_dir2_sf_check(
/* 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 */
@@ -672,12 +685,12 @@ xfs_dir2_sf_verify(
* 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
@@ -685,33 +698,39 @@ xfs_dir2_sf_verify(
* 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;
}
--- a/fs/xfs/libxfs/xfs_inode_fork.c
+++ b/fs/xfs/libxfs/xfs_inode_fork.c
@@ -212,6 +212,16 @@ 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);
@@ -322,8 +332,6 @@ xfs_iformat_local(
int whichfork,
int size)
{
- int error;
-
/*
* If the size is unreasonable, then something
* is wrong and we just bail out rather than crash in
@@ -339,14 +347,6 @@ 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;
}
@@ -867,7 +867,7 @@ xfs_iextents_copy(
* 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,
@@ -877,7 +877,6 @@ 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] =
@@ -886,7 +885,7 @@ xfs_iflush_fork(
{ 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,
@@ -894,19 +893,12 @@ xfs_iflush_fork(
*/
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);
@@ -959,7 +951,6 @@ 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 *);
-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);
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -50,6 +50,7 @@
#include "xfs_log.h"
#include "xfs_bmap_btree.h"
#include "xfs_reflink.h"
+#include "xfs_dir2_priv.h"
kmem_zone_t *xfs_inode_zone;
@@ -3491,7 +3492,6 @@ 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));
@@ -3563,6 +3563,12 @@ 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))
+ goto corrupt_out;
+
/*
* 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
@@ -3574,14 +3580,9 @@ xfs_iflush_int(
if (ip->i_d.di_flushiter == DI_MAX_FLUSH)
ip->i_d.di_flushiter = 0;
- 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_iflush_fork(ip, dip, iip, XFS_DATA_FORK);
+ if (XFS_IFORK_Q(ip))
+ xfs_iflush_fork(ip, dip, iip, XFS_ATTR_FORK);
xfs_inobp_check(mp, bp);
/*
Patches currently in stable-queue which might be from darrick.wong@oracle.com are
queue-4.9/xfs-fix-missed-holes-in-seek_hole-implementation.patch
queue-4.9/xfs-fix-off-by-one-on-max-nr_pages-in-xfs_find_get_desired_pgoff.patch
queue-4.9/xfs-use-dedicated-log-worker-wq-to-avoid-deadlock-with-cil-wq.patch
queue-4.9/xfs-rework-the-inline-directory-verifiers.patch
queue-4.9/xfs-use-b_state-to-fix-buffer-i-o-accounting-release-race.patch
queue-4.9/xfs-verify-inline-directory-data-forks.patch
queue-4.9/xfs-fix-kernel-memory-exposure-problems.patch
reply other threads:[~2017-06-05 14:09 UTC|newest]
Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=149667174215513@kroah.com \
--to=gregkh@linuxfoundation.org \
--cc=bfoster@redhat.com \
--cc=darrick.wong@oracle.com \
--cc=hch@lst.de \
--cc=stable-commits@vger.kernel.org \
--cc=stable@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.