From: Brian Foster <bfoster@redhat.com>
To: linux-xfs@vger.kernel.org
Subject: [PATCH RFC] xfs: fix buffer check for primary sb in userspace libxfs
Date: Tue, 18 Jul 2017 10:13:37 -0400 [thread overview]
Message-ID: <20170718141337.46255-1-bfoster@redhat.com> (raw)
Signed-off-by: Brian Foster <bfoster@redhat.com>
---
Hi all,
This patch is actually targeted at userspace. The previous change in commit
f3d7ebde ("xfs: fix superblock inprogress check") to use ->b_maps technically
breaks the logic in userspace in a similar way to the original problem because
userspace has no concept of uncached buffers. ->b_maps is NULL in userspace
unless the buffer is truly discontiguous.
This would normally result in a segfault but this appears to be hidden
by gcc optimization as -O2 is enabled by default and the
check_inprogress param to xfs_mount_validate_sb() is unused in
userspace. Therefore, the segfault is only reproducible when
optimization is disabled (which is a useful configuration for
debugging).
There are obviously different ways to fix this. I'm floating this (untested)
rfc as a kernel patch (do we ever sync libxfs from xfsprogs -> kernel?) with
the objective of keeping the libxfs code the same between the kernel and
userspace. We could alternatively create a custom helper/macro with the
appropriate check in each place. Thoughts?
Brian
fs/xfs/libxfs/xfs_sb.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c
index 9b5aae2..ec2fd03 100644
--- a/fs/xfs/libxfs/xfs_sb.c
+++ b/fs/xfs/libxfs/xfs_sb.c
@@ -583,6 +583,7 @@ xfs_sb_verify(
{
struct xfs_mount *mp = bp->b_target->bt_mount;
struct xfs_sb sb;
+ bool primary_sb;
/*
* Use call variant which doesn't convert quota flags from disk
@@ -592,11 +593,14 @@ xfs_sb_verify(
/*
* Only check the in progress field for the primary superblock as
- * mkfs.xfs doesn't clear it from secondary superblocks.
+ * mkfs.xfs doesn't clear it from secondary superblocks. Note that
+ * userspace libxfs does not have uncached buffers and so b_maps is not
+ * used for the sb buffer.
*/
- return xfs_mount_validate_sb(mp, &sb,
- bp->b_maps[0].bm_bn == XFS_SB_DADDR,
- check_version);
+ primary_sb = (bp->b_bn == XFS_BUF_DADDR_NULL &&
+ bp->b_maps[0].bm_bn == XFS_SB_DADDR) ||
+ bp->b_bn == XFS_SB_DADDR;
+ return xfs_mount_validate_sb(mp, &sb, primary_sb, check_version);
}
/*
--
2.9.4
next reply other threads:[~2017-07-18 14:13 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-07-18 14:13 Brian Foster [this message]
2017-07-18 18:10 ` [PATCH RFC] xfs: fix buffer check for primary sb in userspace libxfs Darrick J. Wong
2017-07-18 18:23 ` Brian Foster
2017-07-18 23:12 ` Dave Chinner
2017-07-19 11:17 ` Brian Foster
2017-07-20 2:48 ` Dave Chinner
2017-07-20 11:52 ` Brian Foster
2017-08-16 6:22 ` Darrick J. Wong
2017-08-16 10:31 ` Brian Foster
2017-08-16 15:22 ` Darrick J. Wong
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=20170718141337.46255-1-bfoster@redhat.com \
--to=bfoster@redhat.com \
--cc=linux-xfs@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.