From: Murphy Zhou <xzhou@redhat.com>
To: Eric Biggers <ebiggers@kernel.org>
Cc: fstests@vger.kernel.org, linux-fscrypt@vger.kernel.org,
keyrings@vger.kernel.org, Murphy Zhou <xzhou@redhat.com>
Subject: Re: [PATCH] generic/581: try to avoid flakiness in keys quota test
Date: Wed, 5 Feb 2020 12:39:05 +0800 [thread overview]
Message-ID: <20200205043905.xpp5rniupbi4j7qa@xzhoux.usersys.redhat.com> (raw)
In-Reply-To: <20200129004251.133747-1-ebiggers@kernel.org>
On Tue, Jan 28, 2020 at 04:42:51PM -0800, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
>
> generic/581 passes for me, but Murphy Zhou reported that it started
> failing for him. The part that failed is the part that sets the key
> quota to the fsgqa user's current number of keys plus 5, then tries to
> add 6 filesystem encryption keys as the fsgqa user. Adding the 6th key
> unexpectedly succeeded.
Ack. This patch fixed generic/581 failure on my side.
Thanks Eric!
Murphy
>
> What I think is happening is that because the kernel's keys subsystem
> garbage-collects keys asynchronously, the quota may be freed up later
> than expected after removing fscrypt keys. Thus the test is flaky.
>
> It would be nice to fix this in the kernel, but unfortunately there
> doesn't seem to be an easy fix, and the keys subsystem has always worked
> this way. And it seems unlikely to cause real-world problems, as the
> keys quota really just exists to prevent denial-of-service attacks.
>
> So, for now just try to make the test more reliable by:
>
> (1) Reduce the scope of the modified keys quota to just the part of the
> test that needs it.
> (2) Before getting the current number of keys for the purpose of setting
> the quota, wait for any invalidated keys to be garbage-collected.
>
> Tested with a kernel that has a 1 second sleep hacked into the beginning
> of key_garbage_collector(). With that, this test fails before this
> patch and passes afterwards.
>
> Reported-by: Murphy Zhou <xzhou@redhat.com>
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
> tests/generic/581 | 29 +++++++++++++++++++++--------
> 1 file changed, 21 insertions(+), 8 deletions(-)
>
> diff --git a/tests/generic/581 b/tests/generic/581
> index 89aa03c2..bc49eadc 100755
> --- a/tests/generic/581
> +++ b/tests/generic/581
> @@ -45,14 +45,6 @@ _require_scratch_encryption -v 2
> _scratch_mkfs_encrypted &>> $seqres.full
> _scratch_mount
>
> -# Set the fsgqa user's key quota to their current number of keys plus 5.
> -orig_keys=$(_user_do "awk '/^[[:space:]]*$(id -u fsgqa):/{print \$4}' /proc/key-users | cut -d/ -f1")
> -: ${orig_keys:=0}
> -echo "orig_keys=$orig_keys" >> $seqres.full
> -orig_maxkeys=$(</proc/sys/kernel/keys/maxkeys)
> -keys_to_add=5
> -echo $((orig_keys + keys_to_add)) > /proc/sys/kernel/keys/maxkeys
> -
> dir=$SCRATCH_MNT/dir
>
> raw_key=""
> @@ -98,6 +90,24 @@ _user_do_rm_enckey $SCRATCH_MNT $keyid
>
> _scratch_cycle_mount # Clear all keys
>
> +# Wait for any invalidated keys to be garbage-collected.
> +i=0
> +while grep -E -q '^[0-9a-f]+ [^ ]*i[^ ]*' /proc/keys; do
> + if ((++i >= 20)); then
> + echo "Timed out waiting for invalidated keys to be GC'ed" >> $seqres.full
> + break
> + fi
> + sleep 0.5
> +done
> +
> +# Set the user key quota to the fsgqa user's current number of keys plus 5.
> +orig_keys=$(_user_do "awk '/^[[:space:]]*$(id -u fsgqa):/{print \$4}' /proc/key-users | cut -d/ -f1")
> +: ${orig_keys:=0}
> +echo "orig_keys=$orig_keys" >> $seqres.full
> +orig_maxkeys=$(</proc/sys/kernel/keys/maxkeys)
> +keys_to_add=5
> +echo $((orig_keys + keys_to_add)) > /proc/sys/kernel/keys/maxkeys
> +
> echo
> echo "# Testing user key quota"
> for i in `seq $((keys_to_add + 1))`; do
> @@ -106,6 +116,9 @@ for i in `seq $((keys_to_add + 1))`; do
> | sed 's/ with identifier .*$//'
> done
>
> +# Restore the original key quota.
> +echo "$orig_maxkeys" > /proc/sys/kernel/keys/maxkeys
> +
> rm -rf $dir
> echo
> _user_do "mkdir $dir"
> --
> 2.25.0.341.g760bfbb309-goog
>
prev parent reply other threads:[~2020-02-05 4:39 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-01-29 0:42 [PATCH] generic/581: try to avoid flakiness in keys quota test Eric Biggers
2020-02-05 4:39 ` Murphy Zhou [this message]
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=20200205043905.xpp5rniupbi4j7qa@xzhoux.usersys.redhat.com \
--to=xzhou@redhat.com \
--cc=ebiggers@kernel.org \
--cc=fstests@vger.kernel.org \
--cc=keyrings@vger.kernel.org \
--cc=linux-fscrypt@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).