[PATCH] btrfs-progs: exit with failure when printing bad superblock

[PATCH] btrfs-progs: exit with failure when printing bad superblock

[Mike Fleetwood]

Attempting to dump a bad btrfs superblock returns successful exit status
zero.  According to the manual page non-zero should be returned on
failure.  Fix this.
    $ btrfs inspect-internal dump-super /dev/zero
    superblock: bytenr=65536, device=/dev/zero
    ---------------------------------------------------------
    ERROR: bad magic on superblock on /dev/zero at 65536

    $ echo $?
    0

Signed-off-by: Mike Fleetwood <mike.fleetwood@googlemail.com>
---
 cmds/inspect-dump-super.c                       | 11 ++++++++---
 tests/misc-tests/015-dump-super-garbage/test.sh |  6 +++---
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/cmds/inspect-dump-super.c b/cmds/inspect-dump-super.c
index d843562..4187da8 100644
--- a/cmds/inspect-dump-super.c
+++ b/cmds/inspect-dump-super.c
@@ -52,9 +52,9 @@ static int load_and_dump_sb(char *filename, int fd, u64 sb_bytenr, int full,
 	if (btrfs_super_magic(&sb) != BTRFS_MAGIC && !force) {
 		error("bad magic on superblock on %s at %llu",
 				filename, (unsigned long long)sb_bytenr);
-	} else {
-		btrfs_print_superblock(&sb, full);
+		return 1;
 	}
+	btrfs_print_superblock(&sb, full);
 	return 0;
 }
 
@@ -177,7 +177,12 @@ static int cmd_inspect_dump_super(const struct cmd_struct *cmd,
 				putchar('\n');
 			}
 		} else {
-			load_and_dump_sb(filename, fd, sb_bytenr, full, force);
+			if (load_and_dump_sb(filename, fd,
+						sb_bytenr, full, force)) {
+				close(fd);
+				ret = 1;
+				goto out;
+			}
 			putchar('\n');
 		}
 		close(fd);
diff --git a/tests/misc-tests/015-dump-super-garbage/test.sh b/tests/misc-tests/015-dump-super-garbage/test.sh
index b346945..1e6afa1 100755
--- a/tests/misc-tests/015-dump-super-garbage/test.sh
+++ b/tests/misc-tests/015-dump-super-garbage/test.sh
@@ -6,9 +6,9 @@ source "$TEST_TOP/common"
 
 check_prereq btrfs
 
-run_check "$TOP/btrfs" inspect-internal dump-super /dev/urandom
-run_check "$TOP/btrfs" inspect-internal dump-super -a /dev/urandom
-run_check "$TOP/btrfs" inspect-internal dump-super -fa /dev/urandom
+run_mustfail "attempt to print bad superblock without force" "$TOP/btrfs" inspect-internal dump-super /dev/urandom
+run_mustfail "attempt to print bad superblock without force" "$TOP/btrfs" inspect-internal dump-super -a /dev/urandom
+run_mustfail "attempt to print bad superblock without force" "$TOP/btrfs" inspect-internal dump-super -fa /dev/urandom
 run_check "$TOP/btrfs" inspect-internal dump-super -Ffa /dev/urandom
 run_check "$TOP/btrfs" inspect-internal dump-super -Ffa /dev/urandom
 run_check "$TOP/btrfs" inspect-internal dump-super -Ffa /dev/urandom
-- 
1.8.3.1

Re: [PATCH] btrfs-progs: exit with failure when printing bad superblock

[David Sterba]

On Sun, Jul 24, 2022 at 04:32:14PM +0100, Mike Fleetwood wrote:
> Attempting to dump a bad btrfs superblock returns successful exit status
> zero.  According to the manual page non-zero should be returned on
> failure.  Fix this.
>     $ btrfs inspect-internal dump-super /dev/zero
>     superblock: bytenr=65536, device=/dev/zero
>     ---------------------------------------------------------
>     ERROR: bad magic on superblock on /dev/zero at 65536
> 
>     $ echo $?
>     0
> 
> Signed-off-by: Mike Fleetwood <mike.fleetwood@googlemail.com>

Makes sense, thanks.

> ---
>  cmds/inspect-dump-super.c                       | 11 ++++++++---
>  tests/misc-tests/015-dump-super-garbage/test.sh |  6 +++---
>  2 files changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/cmds/inspect-dump-super.c b/cmds/inspect-dump-super.c
> index d843562..4187da8 100644
> --- a/cmds/inspect-dump-super.c
> +++ b/cmds/inspect-dump-super.c
> @@ -52,9 +52,9 @@ static int load_and_dump_sb(char *filename, int fd, u64 sb_bytenr, int full,
>  	if (btrfs_super_magic(&sb) != BTRFS_MAGIC && !force) {
>  		error("bad magic on superblock on %s at %llu",

I've added a notice to use --force to show it.

>  				filename, (unsigned long long)sb_bytenr);
> -	} else {
> -		btrfs_print_superblock(&sb, full);
> +		return 1;
>  	}
> +	btrfs_print_superblock(&sb, full);
>  	return 0;
>  }
>  
> @@ -177,7 +177,12 @@ static int cmd_inspect_dump_super(const struct cmd_struct *cmd,
>  				putchar('\n');
>  			}
>  		} else {
> -			load_and_dump_sb(filename, fd, sb_bytenr, full, force);
> +			if (load_and_dump_sb(filename, fd,
> +						sb_bytenr, full, force)) {
> +				close(fd);
> +				ret = 1;
> +				goto out;
> +			}
>  			putchar('\n');
>  		}
>  		close(fd);
> diff --git a/tests/misc-tests/015-dump-super-garbage/test.sh b/tests/misc-tests/015-dump-super-garbage/test.sh
> index b346945..1e6afa1 100755
> --- a/tests/misc-tests/015-dump-super-garbage/test.sh
> +++ b/tests/misc-tests/015-dump-super-garbage/test.sh
> @@ -6,9 +6,9 @@ source "$TEST_TOP/common"
>  
>  check_prereq btrfs
>  
> -run_check "$TOP/btrfs" inspect-internal dump-super /dev/urandom
> -run_check "$TOP/btrfs" inspect-internal dump-super -a /dev/urandom
> -run_check "$TOP/btrfs" inspect-internal dump-super -fa /dev/urandom
> +run_mustfail "attempt to print bad superblock without force" "$TOP/btrfs" inspect-internal dump-super /dev/urandom
> +run_mustfail "attempt to print bad superblock without force" "$TOP/btrfs" inspect-internal dump-super -a /dev/urandom
> +run_mustfail "attempt to print bad superblock without force" "$TOP/btrfs" inspect-internal dump-super -fa /dev/urandom
>  run_check "$TOP/btrfs" inspect-internal dump-super -Ffa /dev/urandom
>  run_check "$TOP/btrfs" inspect-internal dump-super -Ffa /dev/urandom
>  run_check "$TOP/btrfs" inspect-internal dump-super -Ffa /dev/urandom
> -- 
> 1.8.3.1