| Message ID | 20200429194420.21147-5-dbuono@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add support for SafeStack | expand |
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 <dbuono@linux.vnet.ibm.com> > --- > 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
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 <dbuono@linux.vnet.ibm.com> >> --- >> 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 >
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 <dbuono@linux.vnet.ibm.com> > > > --- > > > 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
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
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. Signed-off-by: Daniele Buono <dbuono@linux.vnet.ibm.com> --- tests/check-block.sh | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)