linux-ext4 - Re: [PATCH] shared/272: fail quickly on mkfs errors and improve logging

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20141003043611.GK24490@dastard>
Date:	Fri, 3 Oct 2014 14:36:11 +1000
From:	Dave Chinner <david@...morbit.com>
To:	Eric Whitney <enwlinux@...il.com>
Cc:	fstests@...r.kernel.org, linux-ext4@...r.kernel.org
Subject: Re: [PATCH] shared/272: fail quickly on mkfs errors and improve
 logging

On Tue, Sep 30, 2014 at 03:50:04PM -0400, Eric Whitney wrote:
> 272 will log diagnostic information if it fails to make its scratch file
> system, but the test itself won't fail immediately.  If the scratch
> device had previously contained a valid filesystem, and the attempt to
> make a small scratch file system on it fails, 272 will mount and run on
> the pre-existing file system (as seen during ext4 inline data testing).
> Since 272 tests to ENOSPC, it can take a long time to learn mkfs failed.
> This behavior can also lead to invalid positive test results unless
> 272.full is examined separately.
> 
> Signed-off-by: Eric Whitney <enwlinux@...il.com>
> ---
>  tests/shared/272 | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/shared/272 b/tests/shared/272
> index 4417535..9695e59 100755
> --- a/tests/shared/272
> +++ b/tests/shared/272
> @@ -87,8 +87,11 @@ _supported_os Linux
>  _need_to_be_root
>  _require_scratch
>  
> -_scratch_mkfs_sized $((64 * 1024 * 1024)) >> $seqres.full 2>&1
> -_scratch_mount
> +rm -f $seqres.full
> +
> +_scratch_mkfs_sized $((64 * 1024 * 1024)) >> $seqres.full 2>&1 \
> +	|| _fail "mkfs failed"
> +_scratch_mount >> $seqres.full 2>&1 || _fail "mount failed"

Let's not have an unending stream of patches to make this same
change to every test that calls _scratch_mkfs or scratch_mount.

If you need more debug output from them if they fail, please put it
inside the low level _scratch_mkfs_$FSTYP functions themselves, as
they already capture errors and dump debug to $seqres.full.

And we've had the discussion in the past about failing if
scratch_mkfs fails. It came down on the side of "unless there's a
specific reason for failing the test, it should still run to give us
as much coverage as possible". e.g  a failed mkfs can result in the
test exercising error paths being exercised that wouldn't otherwise
be tested....

The case you have here (filling the fs can take ages) is a good
reason for terminating the test if _scratch_mkfs_sized fails, but in
general we really want the tests to run if at all possible...

Cheers,

Dave.
-- 
Dave Chinner
david@...morbit.com
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html