* [PATCH] xfs_metadump: Make -F (force) optional
@ 2013-12-12 17:47 Eric Sandeen
2013-12-12 18:31 ` Christoph Hellwig
0 siblings, 1 reply; 4+ messages in thread
From: Eric Sandeen @ 2013-12-12 17:47 UTC (permalink / raw)
To: xfs-oss, Christoph Hellwig
If we do something crazy like:
# xfs_metadump /root/anaconda.cfg outfile
xfs_metadump will pass "-F" to xfs_db to carry on even in the face
of bad superblock magic. [1] Depending on what we gave as an input
file, we may very well fail quite badly:
> xfs_metadump: /root/anaconda.cfg is not a valid XFS filesystem (unexpected SB magic number 0x230a2320)
> Floating point exception
I don't think it's possible to harden every path through libxfs for
non-xfs filesystems as input. (Even if it's possible, I don't think it's
worth the effort).
So I propose making the "-F" optional; by default, xfs_metadump will
say no, this has bad magic, I'm stopping. If the admin really wants to
try to proceed, suggest that they can use "-F" and they can keep all the
broken pieces.
[1] behavior added in 7f98455 xfs_db: exit on invalid magic number
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
diff --git a/db/init.c b/db/init.c
index 8f86f45..e7f536a 100644
--- a/db/init.c
+++ b/db/init.c
@@ -140,8 +140,10 @@ init(
if (sbp->sb_magicnum != XFS_SB_MAGIC) {
fprintf(stderr, _("%s: %s is not a valid XFS filesystem (unexpected SB magic number 0x%08x)\n"),
progname, fsdevice, sbp->sb_magicnum);
- if (!force)
+ if (!force) {
+ fprintf(stderr, _("Use -F to force a read attempt.\n"));
exit(EXIT_FAILURE);
+ }
}
mp = libxfs_mount(&xmount, sbp, x.ddev, x.logdev, x.rtdev,
diff --git a/db/xfs_metadump.sh b/db/xfs_metadump.sh
index 28b04b8..7ce7a6d 100755
--- a/db/xfs_metadump.sh
+++ b/db/xfs_metadump.sh
@@ -5,7 +5,7 @@
OPTS=" "
DBOPTS=" "
-USAGE="Usage: xfs_metadump [-efogwV] [-m max_extents] [-l logdev] source target"
+USAGE="Usage: xfs_metadump [-efFogwV] [-m max_extents] [-l logdev] source target"
while getopts "efgl:m:owV" c
do
@@ -17,6 +17,7 @@ do
w) OPTS=$OPTS"-w ";;
f) DBOPTS=$DBOPTS" -f";;
l) DBOPTS=$DBOPTS" -l "$OPTARG" ";;
+ F) DBOPTS=$DBOPTS" -F";;
V) xfs_db -p xfs_metadump -V
status=$?
exit $status
@@ -29,7 +30,7 @@ done
set -- extra $@
shift $OPTIND
case $# in
- 2) xfs_db$DBOPTS -F -i -p xfs_metadump -c "metadump$OPTS $2" $1
+ 2) xfs_db$DBOPTS -i -p xfs_metadump -c "metadump$OPTS $2" $1
status=$?
;;
*) echo $USAGE 1>&2
diff --git a/man/man8/xfs_metadump.8 b/man/man8/xfs_metadump.8
index 4fa1b1c..077fff5 100644
--- a/man/man8/xfs_metadump.8
+++ b/man/man8/xfs_metadump.8
@@ -4,7 +4,7 @@ xfs_metadump \- copy XFS filesystem metadata to a file
.SH SYNOPSIS
.B xfs_metadump
[
-.B \-efgow
+.B \-efFgow
] [
.B \-m
.I max_extents
@@ -86,6 +86,11 @@ file option). This can also happen if an image copy of a filesystem has
been made into an ordinary file with
.BR xfs_copy (8).
.TP
+.B \-F
+Specifies that we want to continue even if the superblock magic is not correct.
+If the source is truly not an XFS filesystem, the resulting image will be useless,
+and xfs_metadump may crash.
+.TP
.B \-g
Shows dump progress. This is sent to stdout if the
.I target
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] xfs_metadump: Make -F (force) optional
2013-12-12 17:47 [PATCH] xfs_metadump: Make -F (force) optional Eric Sandeen
@ 2013-12-12 18:31 ` Christoph Hellwig
2013-12-12 20:01 ` Eric Sandeen
0 siblings, 1 reply; 4+ messages in thread
From: Christoph Hellwig @ 2013-12-12 18:31 UTC (permalink / raw)
To: Eric Sandeen; +Cc: Christoph Hellwig, xfs-oss
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
> xfs_metadump will pass "-F" to xfs_db to carry on even in the face
> of bad superblock magic. [1] Depending on what we gave as an input
> file, we may very well fail quite badly:
>
> > xfs_metadump: /root/anaconda.cfg is not a valid XFS filesystem (unexpected SB magic number 0x230a2320)
> > Floating point exception
>
> I don't think it's possible to harden every path through libxfs for
> non-xfs filesystems as input. (Even if it's possible, I don't think it's
> worth the effort).
Can you add a test to verify that xfs_metadump now correctly rejects
files that don't have a valid superblock?
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] xfs_metadump: Make -F (force) optional
2013-12-12 18:31 ` Christoph Hellwig
@ 2013-12-12 20:01 ` Eric Sandeen
2013-12-17 16:55 ` Rich Johnston
0 siblings, 1 reply; 4+ messages in thread
From: Eric Sandeen @ 2013-12-12 20:01 UTC (permalink / raw)
To: Christoph Hellwig, Eric Sandeen; +Cc: xfs-oss
On 12/12/13, 12:31 PM, Christoph Hellwig wrote:
> Looks good,
>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
>
>> xfs_metadump will pass "-F" to xfs_db to carry on even in the face
>> of bad superblock magic. [1] Depending on what we gave as an input
>> file, we may very well fail quite badly:
>>
>>> xfs_metadump: /root/anaconda.cfg is not a valid XFS filesystem (unexpected SB magic number 0x230a2320)
>>> Floating point exception
>>
>> I don't think it's possible to harden every path through libxfs for
>> non-xfs filesystems as input. (Even if it's possible, I don't think it's
>> worth the effort).
>
> Can you add a test to verify that xfs_metadump now correctly rejects
> files that don't have a valid superblock?
ok, can do.
Thanks Christoph,
-Eric
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs
>
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] xfs_metadump: Make -F (force) optional
2013-12-12 20:01 ` Eric Sandeen
@ 2013-12-17 16:55 ` Rich Johnston
0 siblings, 0 replies; 4+ messages in thread
From: Rich Johnston @ 2013-12-17 16:55 UTC (permalink / raw)
To: Eric Sandeen, Christoph Hellwig, Eric Sandeen; +Cc: xfs-oss
This has been committed.
Thanks
--Rich
commit 4e83ac7b7416edb52815c6e5f4ca64dadfa5f974
Author: Eric Sandeen <sandeen@redhat.com>
Date: Thu Dec 12 17:47:59 2013 +0000
xfs_metadump: Make -F (force) optional
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-12-17 16:55 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-12 17:47 [PATCH] xfs_metadump: Make -F (force) optional Eric Sandeen
2013-12-12 18:31 ` Christoph Hellwig
2013-12-12 20:01 ` Eric Sandeen
2013-12-17 16:55 ` Rich Johnston
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox