11:28 a.m.
This changes the summary output to report whether or not the
./configure --enable-libfuzzer flag was set:
libfuzzer .............................. yes
or for --disable-libfuzzer / the default of not set:
libfuzzer .............................. no
---
configure.ac | 2 ++
1 file changed, 2 insertions(+)
diff --git a/configure.ac b/configure.ac
index 6a3be4ec..f86215d9 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1417,6 +1417,8 @@ echo "Optional server features:"
echo
feature "bash-completion ........................ " \
test "x$HAVE_BASH_COMPLETION_TRUE" = "x"
+feature "libfuzzer .............................. " \
+ test "x$ENABLE_LIBFUZZER_TRUE" = "x"
feature "manual pages ........................... " \
test "x$HAVE_POD_TRUE" = "x"
feature "SELinux ................................ " \
--
2.35.1
11:28 a.m.
New subject: [PATCH nbdkit 2/3] configure: Add --disable-linker-script flag
This flag allows you to disable the linker script (for the server
only). This is necessary for ASAN to work as explained in the
documentation.
Thanks Eric Blake for helping me with working out the root cause.
---
configure.ac | 30 ++++++++++++++++++++++++------
server/Makefile.am | 5 -----
README | 13 +++++++++++++
3 files changed, 37 insertions(+), 11 deletions(-)
diff --git a/configure.ac b/configure.ac
index f86215d9..404d4e23 100644
--- a/configure.ac
+++ b/configure.ac
@@ -502,17 +502,19 @@ main (int argc, char *argv[])
])
AM_CONDITIONAL([HAVE_ICONV], [test "x$iconv_working" = "xyes"])
+use_linker_script_for_server=yes
+
dnl Don't use linker script for the server on FreeBSD because
dnl FreeBSD's linker is broken. See eg:
dnl https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=190851
-AC_MSG_CHECKING([if we should use a linker script for the server])
+AC_MSG_CHECKING([if we should disable the linker script (FreeBSD only)])
AS_CASE([$host_os],
- [freebsd*], [use_linker_script_for_server=no],
- [use_linker_script_for_server=yes]
+ [freebsd*], [
+ use_linker_script_for_server=no
+ AC_MSG_RESULT([yes])
+ ],
+ [AC_MSG_RESULT([no])]
)
-AC_MSG_RESULT([$use_linker_script_for_server])
-AM_CONDITIONAL([USE_LINKER_SCRIPT_FOR_SERVER],
- [test "x$use_linker_script_for_server" = "xyes"])
dnl Check if -rdynamic linker flag works.
acx_nbdkit_save_LDFLAGS="${LDFLAGS}"
@@ -691,9 +693,23 @@ AC_ARG_ENABLE([libfuzzer],
[enable_libfuzzer=no])
AS_IF([test "x$enable_libfuzzer" = "xyes"],[
AC_DEFINE([ENABLE_LIBFUZZER],[1],[Enable special libFuzzer binary])
+ # We have to disable the linker script for libFuzzer because Clang
+ # adds loads of fuzzer and ASAN-related symbols that are required
+ # by the plugins but which our linker script tries to hide.
+ use_linker_script_for_server=no
])
AM_CONDITIONAL([ENABLE_LIBFUZZER],[test "x$enable_libfuzzer" =
"xyes"])
+dnl Should we use the linker script with the server? Note
+dnl some tests above may set this variable.
+AC_ARG_ENABLE([linker-script],
+ [AS_HELP_STRING([--disable-linker-script],
+ [disable linker script for server (developers only)])],
+ [use_linker_script_for_server=$enableval],
+ [])
+AM_CONDITIONAL([USE_LINKER_SCRIPT_FOR_SERVER],
+ [test "x$use_linker_script_for_server" = "xyes"])
+
dnl Bash completion.
PKG_CHECK_MODULES([BASH_COMPLETION], [bash-completion >= 2.0], [
bash_completion=yes
@@ -1419,6 +1435,8 @@ feature "bash-completion ........................ " \
test "x$HAVE_BASH_COMPLETION_TRUE" = "x"
feature "libfuzzer .............................. " \
test "x$ENABLE_LIBFUZZER_TRUE" = "x"
+feature "linker script .......................... " \
+ test "x$USE_LINKER_SCRIPT_FOR_SERVER_TRUE" = "x"
feature "manual pages ........................... " \
test "x$HAVE_POD_TRUE" = "x"
feature "SELinux ................................ " \
diff --git a/server/Makefile.am b/server/Makefile.am
index 0039d1da..9ab621d5 100644
--- a/server/Makefile.am
+++ b/server/Makefile.am
@@ -108,13 +108,8 @@ nbdkit_LDFLAGS = \
$(NULL)
EXTRA_nbdkit_DEPENDENCIES = nbdkit.syms
if USE_LINKER_SCRIPT_FOR_SERVER
-# We have to disable the linker script for libFuzzer because Clang
-# adds loads of fuzzer and ASAN-related symbols that are required by
-# the plugins but which our linker script tries to hide.
-if !ENABLE_LIBFUZZER
nbdkit_LDFLAGS += -Wl,--version-script=$(srcdir)/nbdkit.syms
endif
-endif
if IS_WINDOWS
# On Windows, generate an import library so that plugins can link
diff --git a/README b/README
index 3480d561..7aa93b1d 100644
--- a/README
+++ b/README
@@ -324,6 +324,19 @@ For further information, see:
https://libguestfs.org/
https://github.com/NetworkBlockDevice/nbd/blob/master/doc/proto.md
+Address sanitizer (ASAN)
+------------------------
+
+You can compile nbdkit with clang and ASAN with:
+
+ ./configure CC=clang CXX=clang++ \
+ CFLAGS="-O0 -g -fsanitize=address -fno-omit-frame-pointer" \
+ --disable-linker-script \
+ --disable-golang
+ make clean
+ make
+ ASAN_OPTIONS="allocator_may_return_null=1 detect_leaks=false" make check
+
Test coverage
-------------
--
2.35.1
4:20 a.m.
New subject: [PATCH nbdkit 2/3] configure: Add --disable-linker-script flag
On 04/06/22 18:28, Richard W.M. Jones wrote:
This flag allows you to disable the linker script (for the server
only). This is necessary for ASAN to work as explained in the
documentation.
Thanks Eric Blake for helping me with working out the root cause.
---
configure.ac | 30 ++++++++++++++++++++++++------
server/Makefile.am | 5 -----
README | 13 +++++++++++++
3 files changed, 37 insertions(+), 11 deletions(-)
diff --git a/configure.ac b/configure.ac
index f86215d9..404d4e23 100644
--- a/configure.ac
+++ b/configure.ac
@@ -502,17 +502,19 @@ main (int argc, char *argv[])
])
AM_CONDITIONAL([HAVE_ICONV], [test "x$iconv_working" = "xyes"])
+use_linker_script_for_server=yes
+
dnl Don't use linker script for the server on FreeBSD because
dnl FreeBSD's linker is broken. See eg:
dnl https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=190851
-AC_MSG_CHECKING([if we should use a linker script for the server])
+AC_MSG_CHECKING([if we should disable the linker script (FreeBSD only)])
AS_CASE([$host_os],
- [freebsd*], [use_linker_script_for_server=no],
- [use_linker_script_for_server=yes]
+ [freebsd*], [
+ use_linker_script_for_server=no
+ AC_MSG_RESULT([yes])
+ ],
+ [AC_MSG_RESULT([no])]
)
-AC_MSG_RESULT([$use_linker_script_for_server])
-AM_CONDITIONAL([USE_LINKER_SCRIPT_FOR_SERVER],
- [test "x$use_linker_script_for_server" = "xyes"])
dnl Check if -rdynamic linker flag works.
acx_nbdkit_save_LDFLAGS="${LDFLAGS}"
Hunk 1.
@@ -691,9 +693,23 @@ AC_ARG_ENABLE([libfuzzer],
[enable_libfuzzer=no])
AS_IF([test "x$enable_libfuzzer" = "xyes"],[
AC_DEFINE([ENABLE_LIBFUZZER],[1],[Enable special libFuzzer binary])
+ # We have to disable the linker script for libFuzzer because Clang
+ # adds loads of fuzzer and ASAN-related symbols that are required
+ # by the plugins but which our linker script tries to hide.
+ use_linker_script_for_server=no
])
AM_CONDITIONAL([ENABLE_LIBFUZZER],[test "x$enable_libfuzzer" =
"xyes"])
Hunk 2.
+dnl Should we use the linker script with the server? Note
+dnl some tests above may set this variable.
+AC_ARG_ENABLE([linker-script],
+ [AS_HELP_STRING([--disable-linker-script],
+ [disable linker script for server (developers only)])],
+ [use_linker_script_for_server=$enableval],
+ [])
Hunk 3.
+AM_CONDITIONAL([USE_LINKER_SCRIPT_FOR_SERVER],
+ [test "x$use_linker_script_for_server" = "xyes"])
+
dnl Bash completion.
PKG_CHECK_MODULES([BASH_COMPLETION], [bash-completion >= 2.0], [
bash_completion=yes
Hunk 4.
@@ -1419,6 +1435,8 @@ feature "bash-completion
........................ " \
test "x$HAVE_BASH_COMPLETION_TRUE" = "x"
feature "libfuzzer .............................. " \
test "x$ENABLE_LIBFUZZER_TRUE" = "x"
+feature "linker script .......................... " \
+ test "x$USE_LINKER_SCRIPT_FOR_SERVER_TRUE" = "x"
feature "manual pages ........................... " \
test "x$HAVE_POD_TRUE" = "x"
feature "SELinux ................................ " \
Hunk 5.
diff --git a/server/Makefile.am b/server/Makefile.am
index 0039d1da..9ab621d5 100644
--- a/server/Makefile.am
+++ b/server/Makefile.am
@@ -108,13 +108,8 @@ nbdkit_LDFLAGS = \
$(NULL)
EXTRA_nbdkit_DEPENDENCIES = nbdkit.syms
if USE_LINKER_SCRIPT_FOR_SERVER
-# We have to disable the linker script for libFuzzer because Clang
-# adds loads of fuzzer and ASAN-related symbols that are required by
-# the plugins but which our linker script tries to hide.
-if !ENABLE_LIBFUZZER
nbdkit_LDFLAGS += -Wl,--version-script=$(srcdir)/nbdkit.syms
endif
-endif
if IS_WINDOWS
# On Windows, generate an import library so that plugins can link
Hunk 6.
diff --git a/README b/README
index 3480d561..7aa93b1d 100644
--- a/README
+++ b/README
@@ -324,6 +324,19 @@ For further information, see:
https://libguestfs.org/
https://github.com/NetworkBlockDevice/nbd/blob/master/doc/proto.md
+Address sanitizer (ASAN)
+------------------------
+
+You can compile nbdkit with clang and ASAN with:
+
+ ./configure CC=clang CXX=clang++ \
+ CFLAGS="-O0 -g -fsanitize=address -fno-omit-frame-pointer" \
+ --disable-linker-script \
+ --disable-golang
+ make clean
+ make
+ ASAN_OPTIONS="allocator_may_return_null=1 detect_leaks=false" make check
+
Test coverage
-------------
Hunk 7.
--*--
I find this patch confusing; IMO it does three things at once.
The first patch should be the reversal of how we set
"use_linker_script_for_server" when building on FreeBSD. This is a
refactoring. This patch should consist of hunks #1, #4 and #5.
The second patch should be related to libfuzzer. It should contain hunks
#2 and #6. It is also a refactoring. Notably though, the comment in hunk
#2 should *not* refer to ASAN, only to libfuzzer.
The third patch should be realted to ASAN. It is a feature. It should
consist of hunks #3 and #7.
Basically we have three separate reasons for disabling the linker script
(building on FreeBSD, using libfuzzer, using clang+ASAN). The patches
should reflect that, in my opinion.
Thanks
Laszlo
4:37 a.m.
New subject: [PATCH nbdkit 2/3] configure: Add --disable-linker-script flag
I have split this into three patches in my local copy, but ...
On Thu, Apr 07, 2022 at 11:20:47AM +0200, Laszlo Ersek wrote:
The second patch should be related to libfuzzer. It should contain
hunks
#2 and #6. It is also a refactoring. Notably though, the comment in hunk
#2 should *not* refer to ASAN, only to libfuzzer.
in the second patch I just copied the existing message. I think it's
OK to keep the ASAN reference since libFuzzer separately recommends
using ASAN (https://www.llvm.org/docs/LibFuzzer.html#fuzzer-usage).
Basically we have three separate reasons for disabling the linker
script
(building on FreeBSD, using libfuzzer, using clang+ASAN). The patches
should reflect that, in my opinion.
Thanks,
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW
7:51 a.m.
New subject: [PATCH nbdkit 2/3] configure: Add --disable-linker-script flag
On 04/07/22 11:37, Richard W.M. Jones wrote:
I have split this into three patches in my local copy, but ...
On Thu, Apr 07, 2022 at 11:20:47AM +0200, Laszlo Ersek wrote:
> The second patch should be related to libfuzzer. It should contain hunks
> #2 and #6. It is also a refactoring. Notably though, the comment in hunk
> #2 should *not* refer to ASAN, only to libfuzzer.
in the second patch I just copied the existing message. I think it's
OK to keep the ASAN reference since libFuzzer separately recommends
using ASAN (https://www.llvm.org/docs/LibFuzzer.html#fuzzer-usage).
Fair enough, thank you!
Laszlo
> Basically we have three separate reasons for disabling the linker
script
> (building on FreeBSD, using libfuzzer, using clang+ASAN). The patches
> should reflect that, in my opinion.
Thanks,
Rich.
11:28 a.m.
New subject: [PATCH nbdkit 3/3] fuzzing: Recommend combining fuzzing with ASAN
ASAN (ie. Address Sanitizer or -fsanitize=address) adds extra checks
for out-of-bounds memory access, use-after-free and other memory
checks. It's useful to combine this with fuzzing.
Fuzzing can normally only detect paths which cause the binary to
crash. But some serious, latent bugs might not cause crashes (eg. a
rogue pointer overwrites another object in memory, but the other
object is not used or not used in a way that will cause a crash).
ASAN turns these kinds of bugs into crashes.
See also:
https://clang.llvm.org/docs/AddressSanitizer.html
https://aflplus.plus/docs/notes_for_asan/
Cherry picked from libnbd commit 43b1b95c981861c5c03cd563cf1b90e1f4c52cf8.
RWMJ: Some modifications were required for fuzzing to work with nbdkit.
---
fuzzing/README | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/fuzzing/README b/fuzzing/README
index eeab9744..b2bc6f08 100644
--- a/fuzzing/README
+++ b/fuzzing/README
@@ -15,6 +15,7 @@ You will need to recompile nbdkit with AFL instrumentation:
To use clang instead (recommended with AFL++):
+ export AFL_USE_ASAN=1
./configure CC=/usr/bin/afl-clang-lto CXX=/usr/bin/afl-clang-lto++
make clean
make
@@ -29,14 +30,16 @@ Master:
mkdir -p fuzzing/sync_dir
export AFL_PRELOAD=./plugins/memory/.libs/nbdkit-memory-plugin.so
- afl-fuzz -i fuzzing/testcase_dir -o fuzzing/sync_dir -m 256 -M fuzz01 \
+ export ASAN_OPTIONS="allocator_may_return_null=1 detect_leaks=false"
+ afl-fuzz -i fuzzing/testcase_dir -o fuzzing/sync_dir -M fuzz01 \
./server/nbdkit -s -t 1 ./plugins/memory/.libs/nbdkit-memory-plugin.so 1M
Slaves:
# replace fuzzNN with fuzz02, fuzz03, etc.
export AFL_PRELOAD=./plugins/memory/.libs/nbdkit-memory-plugin.so
- afl-fuzz -i fuzzing/testcase_dir -o fuzzing/sync_dir -m 256 -S fuzzNN \
+ export ASAN_OPTIONS="allocator_may_return_null=1 detect_leaks=false"
+ afl-fuzz -i fuzzing/testcase_dir -o fuzzing/sync_dir -S fuzzNN \
./server/nbdkit -s -t 1 ./plugins/memory/.libs/nbdkit-memory-plugin.so 1M
Test Coverage
--
2.35.1
4:23 a.m.
New subject: [PATCH nbdkit 3/3] fuzzing: Recommend combining fuzzing with ASAN
On 04/06/22 18:28, Richard W.M. Jones wrote:
ASAN (ie. Address Sanitizer or -fsanitize=address) adds extra checks
for out-of-bounds memory access, use-after-free and other memory
checks. It's useful to combine this with fuzzing.
Fuzzing can normally only detect paths which cause the binary to
crash. But some serious, latent bugs might not cause crashes (eg. a
rogue pointer overwrites another object in memory, but the other
object is not used or not used in a way that will cause a crash).
ASAN turns these kinds of bugs into crashes.
See also:
https://clang.llvm.org/docs/AddressSanitizer.html
https://aflplus.plus/docs/notes_for_asan/
Cherry picked from libnbd commit 43b1b95c981861c5c03cd563cf1b90e1f4c52cf8.
RWMJ: Some modifications were required for fuzzing to work with nbdkit.
---
fuzzing/README | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/fuzzing/README b/fuzzing/README
index eeab9744..b2bc6f08 100644
--- a/fuzzing/README
+++ b/fuzzing/README
@@ -15,6 +15,7 @@ You will need to recompile nbdkit with AFL instrumentation:
To use clang instead (recommended with AFL++):
+ export AFL_USE_ASAN=1
./configure CC=/usr/bin/afl-clang-lto CXX=/usr/bin/afl-clang-lto++
make clean
make
@@ -29,14 +30,16 @@ Master:
mkdir -p fuzzing/sync_dir
export AFL_PRELOAD=./plugins/memory/.libs/nbdkit-memory-plugin.so
- afl-fuzz -i fuzzing/testcase_dir -o fuzzing/sync_dir -m 256 -M fuzz01 \
+ export ASAN_OPTIONS="allocator_may_return_null=1 detect_leaks=false"
+ afl-fuzz -i fuzzing/testcase_dir -o fuzzing/sync_dir -M fuzz01 \
./server/nbdkit -s -t 1 ./plugins/memory/.libs/nbdkit-memory-plugin.so 1M
Slaves:
# replace fuzzNN with fuzz02, fuzz03, etc.
export AFL_PRELOAD=./plugins/memory/.libs/nbdkit-memory-plugin.so
- afl-fuzz -i fuzzing/testcase_dir -o fuzzing/sync_dir -m 256 -S fuzzNN \
+ export ASAN_OPTIONS="allocator_may_return_null=1 detect_leaks=false"
+ afl-fuzz -i fuzzing/testcase_dir -o fuzzing/sync_dir -S fuzzNN \
./server/nbdkit -s -t 1 ./plugins/memory/.libs/nbdkit-memory-plugin.so 1M
Test Coverage
I guess this is OK, but we should explain the removal of the "-m 256"
option in the commit message.
With that:
Acked-by: Laszlo Ersek <lersek(a)redhat.com>
Thanks
Laszlo
4:05 a.m.
On 04/06/22 18:28, Richard W.M. Jones wrote:
This changes the summary output to report whether or not the
./configure --enable-libfuzzer flag was set:
libfuzzer .............................. yes
or for --disable-libfuzzer / the default of not set:
libfuzzer .............................. no
---
configure.ac | 2 ++
1 file changed, 2 insertions(+)
diff --git a/configure.ac b/configure.ac
index 6a3be4ec..f86215d9 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1417,6 +1417,8 @@ echo "Optional server features:"
echo
feature "bash-completion ........................ " \
test "x$HAVE_BASH_COMPLETION_TRUE" = "x"
+feature "libfuzzer .............................. " \
+ test "x$ENABLE_LIBFUZZER_TRUE" = "x"
feature "manual pages ........................... " \
test "x$HAVE_POD_TRUE" = "x"
feature "SELinux ................................ " \
Reviewed-by: Laszlo Ersek <lersek(a)redhat.com>
943
days inactive
944
days old
7 comments
2 participants
participants (2)
-
Laszlo Ersek -
Richard W.M. Jones