Re: [PATCH 4/4] check-block: Enable iotests with SafeStack
On Fri, May 22, 2020 at 11:35:42AM -0400, Daniele Buono wrote:
> On 5/21/2020 5:59 AM, Stefan Hajnoczi wrote:
> > On Wed, Apr 29, 2020 at 03:44:20PM -0400, Daniele Buono wrote:
> >
> > >
> > > Signed-off-by: Daniele Buono
> > > ---
> > > tests/check-block.sh | 12 +++-
> > > 1 file changed, 11 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/tests/check-block.sh b/tests/check-block.sh
> > > index ad320c21ba..8e29c868e5 100755
> > > --- a/tests/check-block.sh
> > > +++ b/tests/check-block.sh
> > > @@ -21,7 +21,17 @@ if grep -q "CONFIG_GPROF=y" config-host.mak
> > > 2>/dev/null ; then
> > > exit 0
> > > fi
> > > -if grep -q "CFLAGS.*-fsanitize" config-host.mak 2>/dev/null ; then
> > > +# Disable tests with any sanitizer except for SafeStack
> > > +CFLAGS=$( grep "CFLAGS.*-fsanitize" config-host.mak 2>/dev/null )
> > > +SANITIZE_FLAGS=""
> > > +#Remove all occurrencies of -fsanitize=safe-stack
> > > +for i in ${CFLAGS}; do
> > > +if [ "${i}" != "-fsanitize=safe-stack" ]; then
> > > +SANITIZE_FLAGS="${SANITIZE_FLAGS} ${i}"
> > > +fi
> > > +done
> > > +if echo ${SANITIZE_FLAGS} | grep -q "\-fsanitize" 2>/dev/null; then
> > > +# Have a sanitize flag that is not allowed, stop
> > > echo "Sanitizers are enabled ==> Not running the qemu-iotests."
> > > exit 0
> > > fi
> >
> > The commit that disabled check-block.sh with sanitizers said:
> >
> >The sanitizers (especially the address sanitizer from Clang) are
> >sometimes printing out warnings or false positives - this spoils
> >the output of the iotests, causing some of the tests to fail.
> >
> > It seems fine to allow SafeStack if check-block.sh currently passes with
> > it enabled. Does it pass and produce no extra output?
> >
> Yes, that was the idea. SafeStack should pass the tests without extra
> output.
>
> It did (pass) on my testing machine. However I don't remember if I did the
> full (slow) check or only the partial one.
>
> Will check again before I submit v2
Great, thanks!
Stefan
signature.asc
Description: PGP signature
Re: [PATCH 4/4] check-block: Enable iotests with SafeStack
On 5/21/2020 5:59 AM, Stefan Hajnoczi wrote:
On Wed, Apr 29, 2020 at 03:44:20PM -0400, Daniele Buono wrote:
SafeStack is a stack protection technique implemented in llvm. It is
enabled with a -fsanitize flag.
iotests are currently disabled when any -fsanitize option is used.
Since SafeStack is useful on production environments, and its
implementation may break the binary, filter it out when the check is
performed, so that if SafeStack was the only -fsanitize option, iotests
are still performed.
I can't parse this sentence. What does "its implementation may break the
binary" mean? Do you mean it's worth running tests with SafeStack
enabled because it exposes failures that go unnoticed without SafeStack?
What I meant is that, without proper changes, SafeStack breaks
co-routines. Since they are heavily used in the io subsystem, this is
probably the best class of tests to make sure co-routines are working
fine with SafeStack.
I initially re-enabled the iotests for my internal testing.
Since I wasn't seeing any issue, I thought it would be useful to keep
running this to make sure future implementations of SafeStack won't
break co-routines again.
Signed-off-by: Daniele Buono
---
tests/check-block.sh | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/tests/check-block.sh b/tests/check-block.sh
index ad320c21ba..8e29c868e5 100755
--- a/tests/check-block.sh
+++ b/tests/check-block.sh
@@ -21,7 +21,17 @@ if grep -q "CONFIG_GPROF=y" config-host.mak 2>/dev/null ;
then
exit 0
fi
-if grep -q "CFLAGS.*-fsanitize" config-host.mak 2>/dev/null ; then
+# Disable tests with any sanitizer except for SafeStack
+CFLAGS=$( grep "CFLAGS.*-fsanitize" config-host.mak 2>/dev/null )
+SANITIZE_FLAGS=""
+#Remove all occurrencies of -fsanitize=safe-stack
+for i in ${CFLAGS}; do
+if [ "${i}" != "-fsanitize=safe-stack" ]; then
+SANITIZE_FLAGS="${SANITIZE_FLAGS} ${i}"
+fi
+done
+if echo ${SANITIZE_FLAGS} | grep -q "\-fsanitize" 2>/dev/null; then
+# Have a sanitize flag that is not allowed, stop
echo "Sanitizers are enabled ==> Not running the qemu-iotests."
exit 0
fi
The commit that disabled check-block.sh with sanitizers said:
The sanitizers (especially the address sanitizer from Clang) are
sometimes printing out warnings or false positives - this spoils
the output of the iotests, causing some of the tests to fail.
It seems fine to allow SafeStack if check-block.sh currently passes with
it enabled. Does it pass and produce no extra output?
Yes, that was the idea. SafeStack should pass the tests without extra
output.
It did (pass) on my testing machine. However I don't remember if I did
the full (slow) check or only the partial one.
Will check again before I submit v2
Stefan
Re: [PATCH 4/4] check-block: Enable iotests with SafeStack
On Wed, Apr 29, 2020 at 03:44:20PM -0400, Daniele Buono wrote:
> SafeStack is a stack protection technique implemented in llvm. It is
> enabled with a -fsanitize flag.
> iotests are currently disabled when any -fsanitize option is used.
> Since SafeStack is useful on production environments, and its
> implementation may break the binary, filter it out when the check is
> performed, so that if SafeStack was the only -fsanitize option, iotests
> are still performed.
I can't parse this sentence. What does "its implementation may break the
binary" mean? Do you mean it's worth running tests with SafeStack
enabled because it exposes failures that go unnoticed without SafeStack?
>
> Signed-off-by: Daniele Buono
> ---
> tests/check-block.sh | 12 +++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/tests/check-block.sh b/tests/check-block.sh
> index ad320c21ba..8e29c868e5 100755
> --- a/tests/check-block.sh
> +++ b/tests/check-block.sh
> @@ -21,7 +21,17 @@ if grep -q "CONFIG_GPROF=y" config-host.mak 2>/dev/null ;
> then
> exit 0
> fi
>
> -if grep -q "CFLAGS.*-fsanitize" config-host.mak 2>/dev/null ; then
> +# Disable tests with any sanitizer except for SafeStack
> +CFLAGS=$( grep "CFLAGS.*-fsanitize" config-host.mak 2>/dev/null )
> +SANITIZE_FLAGS=""
> +#Remove all occurrencies of -fsanitize=safe-stack
> +for i in ${CFLAGS}; do
> +if [ "${i}" != "-fsanitize=safe-stack" ]; then
> +SANITIZE_FLAGS="${SANITIZE_FLAGS} ${i}"
> +fi
> +done
> +if echo ${SANITIZE_FLAGS} | grep -q "\-fsanitize" 2>/dev/null; then
> +# Have a sanitize flag that is not allowed, stop
> echo "Sanitizers are enabled ==> Not running the qemu-iotests."
> exit 0
> fi
The commit that disabled check-block.sh with sanitizers said:
The sanitizers (especially the address sanitizer from Clang) are
sometimes printing out warnings or false positives - this spoils
the output of the iotests, causing some of the tests to fail.
It seems fine to allow SafeStack if check-block.sh currently passes with
it enabled. Does it pass and produce no extra output?
Stefan
signature.asc
Description: PGP signature
