diff mbox series

generic/577: add missing fsverity metadata cleaning

Message ID 20221102165134.169135-1-aalbersh@redhat.com (mailing list archive)
State New, archived
Headers show
Series generic/577: add missing fsverity metadata cleaning | expand

Commit Message

Andrey Albershteyn Nov. 2, 2022, 4:51 p.m. UTC
When fs-verity is enabled on the file, file becomes read-only. In last
check, test tries to empty the file. However, fs-verity denies
opening/writing to file.

Remove file beforehand as in other checks.

Signed-off-by: Andrey Albershteyn <aalbersh@redhat.com>
---
 tests/generic/577 | 1 +
 1 file changed, 1 insertion(+)

Comments

Eric Biggers Nov. 2, 2022, 10:41 p.m. UTC | #1
On Wed, Nov 02, 2022 at 05:51:34PM +0100, Andrey Albershteyn wrote:
> When fs-verity is enabled on the file, file becomes read-only. In last
> check, test tries to empty the file. However, fs-verity denies
> opening/writing to file.
> 
> Remove file beforehand as in other checks.
> 
> Signed-off-by: Andrey Albershteyn <aalbersh@redhat.com>
> ---
>  tests/generic/577 | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tests/generic/577 b/tests/generic/577
> index 98c3888f..916f3be3 100755
> --- a/tests/generic/577
> +++ b/tests/generic/577
> @@ -121,6 +121,7 @@ if _fsv_have_hash_algorithm sha512 $fsv_file; then
>  fi
>  
>  echo -e "\n# Testing empty file"
> +reset_fsv_file
>  echo -n > $fsv_file
>  _fsv_sign $fsv_file $sigfile.emptyfile --key=$keyfile --cert=$certfile | \
>  		_filter_scratch

Thank you for reporting this.  How did you notice this?  This test actually is
currently passing, because there is another test bug that makes
_fsv_have_hash_algorithm always return false here.  That really needs to be
fixed first, as otherwise your fix doesn't really do anything.

- Eric
Andrey Albershteyn Nov. 3, 2022, 10:23 a.m. UTC | #2
On Wed, Nov 02, 2022 at 03:41:38PM -0700, Eric Biggers wrote:
> Thank you for reporting this.  How did you notice this?  This test actually is
> currently passing, because there is another test bug that makes
> _fsv_have_hash_algorithm always return false here.  That really needs to be
> fixed first, as otherwise your fix doesn't really do anything.

Hmm, it fails for me. _fsv_have_hash_algorithm also seems to work
(returns 0), what bug do you mean? Don't see anything obvious.

I was running -g verity on ext4/xfs (I'm working on adding fsverity
support to xfs).
Eric Biggers Nov. 3, 2022, 4:34 p.m. UTC | #3
On Thu, Nov 03, 2022 at 11:23:21AM +0100, Andrey Albershteyn wrote:
> On Wed, Nov 02, 2022 at 03:41:38PM -0700, Eric Biggers wrote:
> > Thank you for reporting this.  How did you notice this?  This test actually is
> > currently passing, because there is another test bug that makes
> > _fsv_have_hash_algorithm always return false here.  That really needs to be
> > fixed first, as otherwise your fix doesn't really do anything.
> 
> Hmm, it fails for me. _fsv_have_hash_algorithm also seems to work
> (returns 0), what bug do you mean? Don't see anything obvious.
> 
> I was running -g verity on ext4/xfs (I'm working on adding fsverity
> support to xfs).

That's exciting that you're working on fsverity support for xfs!  I wasn't aware
that someone was working on that.

If the test is failing even on ext4 for you, then you must have changed
something, either in the kernel or in the tests, that exposed the issue.

Anyway, for _fsv_have_hash_algorithm() to work properly, it needs to set the
fs.verity.require_signatures sysctl to 0 temporarily (if it was 1).  I'll send
out a patch if you don't get to it first.

- Eric
diff mbox series

Patch

diff --git a/tests/generic/577 b/tests/generic/577
index 98c3888f..916f3be3 100755
--- a/tests/generic/577
+++ b/tests/generic/577
@@ -121,6 +121,7 @@  if _fsv_have_hash_algorithm sha512 $fsv_file; then
 fi
 
 echo -e "\n# Testing empty file"
+reset_fsv_file
 echo -n > $fsv_file
 _fsv_sign $fsv_file $sigfile.emptyfile --key=$keyfile --cert=$certfile | \
 		_filter_scratch