Bug 202213
Summary: | bluez trunk tests fail with GCC 9 (or with -fsanitize=address with GCC 9) | ||
---|---|---|---|
Product: | Drivers | Reporter: | Martin Liška (mliska) |
Component: | Bluetooth | Assignee: | linux-bluetooth (linux-bluetooth) |
Status: | NEW --- | ||
Severity: | normal | CC: | luiz.dentz, stefan.seyfried |
Priority: | P1 | ||
Hardware: | All | ||
OS: | Linux | ||
Kernel Version: | master | Subsystem: | |
Regression: | No | Bisected commit-id: |
Description
Martin Liška
2019-01-10 13:45:59 UTC
Can you try again after applying the following patch: https://gist.github.com/Vudentz/eb85c3a3d02b2a9bef74d9d6a110561d I'm trying the patch right now. I needed to massage the patch a bit to apply on bluez 5.50 http://paste.opensuse.org/63512977 this is the patch I used. Builds and works with gcc8, could not really test with gcc9 due to OBS shortcomings but will submit to openSUSE Factory, so it will get to the gcc9 staging area, too. With gcc9 (finally found a way to use it in OBS :-), it asserts: abuild@strolchi:~/rpmbuild/BUILD/bluez-5.50> cat unit/test-sdp.log bluetoothd[3066]: Bluetooth daemon 5.50 len: 7 raw_size: 14 cont_len: 0 ** ERROR:unit/test-sdp.c:258:client_handler: assertion failed: ((size_t) len == rsp_pdu->raw_size + rsp_pdu->cont_len) FAIL unit/test-sdp (exit status: 134) I added a printf before the assert (and shifted it down one line) (In reply to Stefan Seyfried from comment #4) > With gcc9 (finally found a way to use it in OBS :-), it asserts: > > abuild@strolchi:~/rpmbuild/BUILD/bluez-5.50> cat unit/test-sdp.log > bluetoothd[3066]: Bluetooth daemon 5.50 > len: 7 raw_size: 14 cont_len: 0 > ** > ERROR:unit/test-sdp.c:258:client_handler: assertion failed: ((size_t) len == > rsp_pdu->raw_size + rsp_pdu->cont_len) > FAIL unit/test-sdp (exit status: 134) > > I added a printf before the assert (and shifted it down one line) The patch does not fix the root problem. #define define_test(name, _mtu, args...) \ do { \ const struct sdp_pdu pdus[] = { \ args, { } \ }; \ static struct test_data data; \ data.mtu = _mtu; \ data.pdu_list = g_memdup(pdus, sizeof(pdus)); \ tester_add(name, &data, NULL, test_sdp, NULL); \ } while (0) here you copy pdus, but you should also memdup .raw_data, otherwise it will reach it's end of scope. Slightly reduced test-case that illustrates that: $ cat test-sdp.i struct a { void *b; long c }; enum { d = 5 } typedef *e; e g_malloc0_n(); typedef enum { f, g } h; *g_io_channel_unix_new(); e g_memdup(); struct i { _Bool j; void *k; long l }; struct m { struct i *n }; struct context { int o; int fd; struct m *data }; int q; struct i r; struct a s[]; t(e u) { struct context *context = u; r = context->data->n[q]; s[0].b = r.k; s[0].c = r.l; writev(context->fd, s, 2); return 0; } v(int channel, h cond, e u) { struct context *context = u; g_source_remove(context->o); g_free(u); tester_test_passed(); } int *w; int aa[]; *x(data) { struct context *context = g_malloc0_n(1, sizeof(struct context)); socketpair(1, d, 0, aa); w = g_io_channel_unix_new(aa[0]); context->o = g_io_add_watch(w, g, v, context); context->fd = aa[1]; context->data = data; } y() { struct context *context = x(); g_idle_add(t, context); } z; main() { tester_init(z); { struct i ab[] = {.1, (char[]){4, 11, 0, 1}, sizeof(0)}; static struct m data; data.n = g_memdup(ab, sizeof(ab)); tester_add("", &data, 0, y); } tester_run(); } $ ./test - init - setup - setup complete - run ================================================================= ==29724==ERROR: AddressSanitizer: stack-use-after-scope on address 0x7fffffffdc00 at pc 0x7ffff744c678 bp 0x7fffffffd9e0 sp 0x7fffffffd190 READ of size 4 at 0x7fffffffdc00 thread T0 #0 0x7ffff744c677 in read_iovec /home/marxin/Programming/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:956 #1 0x7ffff744cded in __interceptor_writev /home/marxin/Programming/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:1150 #2 0x408160 in t /home/marxin/BIG/osc/Base:System/bluez/bluez-5.50/xxx/test-sdp.i:31 #3 0x7ffff7ed8626 (/usr/lib64/libglib-2.0.so.0+0x4d626) #4 0x7ffff7edbc14 in g_main_context_dispatch (/usr/lib64/libglib-2.0.so.0+0x50c14) #5 0x7ffff7edbfd7 (/usr/lib64/libglib-2.0.so.0+0x50fd7) #6 0x7ffff7edc2d1 in g_main_loop_run (/usr/lib64/libglib-2.0.so.0+0x512d1) #7 0x41ad10 in tester_run src/shared/tester.c:830 #8 0x408603 in main /home/marxin/BIG/osc/Base:System/bluez/bluez-5.50/xxx/test-sdp.i:63 #9 0x7ffff7018fea in __libc_start_main ../csu/libc-start.c:308 #10 0x403789 in _start (/home/marxin/BIG/osc/Base:System/bluez/bluez-5.50/xxx/test+0x403789) Address 0x7fffffffdc00 is located in stack of thread T0 at offset 48 in frame #0 0x408394 in main /home/marxin/BIG/osc/Base:System/bluez/bluez-5.50/xxx/test-sdp.i:55 This frame has 2 object(s): [48, 52) '<unknown>' <== Memory access at offset 48 is inside this variable [64, 88) 'ab' (line 58) HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork (longjmp and C++ exceptions *are* supported) SUMMARY: AddressSanitizer: stack-use-after-scope /home/marxin/Programming/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:956 in read_iovec Shadow bytes around the buggy address: 0x10007fff7b30: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x10007fff7b40: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x10007fff7b50: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x10007fff7b60: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x10007fff7b70: 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1 f1 f1 =>0x10007fff7b80:[f8]f2 f8 f8 f8 f3 f3 f3 f3 f3 00 00 00 00 00 00 0x10007fff7b90: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x10007fff7ba0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x10007fff7bb0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x10007fff7bc0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x10007fff7bd0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 So something like the following is required: @@ -59,7 +60,7 @@ struct test_data { #define raw_pdu(args...) \ { \ .valid = true, \ - .raw_data = raw_data(args), \ + .raw_data = g_memdup(raw_data(args), sizeof(raw_data(args))), \ .raw_size = sizeof(raw_data(args)), \ } Most test actually build packets like that so I guess all of them are affected, is there any particular reason why this behavior has changed in GCC 9? The test works for me with: diff --git a/unit/test-sdp.c b/unit/test-sdp.c index 5a50cbbf1..ebe00571d 100644 --- a/unit/test-sdp.c +++ b/unit/test-sdp.c @@ -59,14 +59,14 @@ struct test_data { #define raw_pdu(args...) \ { \ .valid = true, \ - .raw_data = raw_data(args), \ + .raw_data = g_memdup(raw_data(args), sizeof(raw_data(args))), \ .raw_size = sizeof(raw_data(args)), \ } #define raw_pdu_cont(cont, args...) \ { \ .valid = true, \ - .raw_data = raw_data(args), \ + .raw_data = g_memdup(raw_data(args), sizeof(raw_data(args))), \ .raw_size = sizeof(raw_data(args)), \ .cont_len = cont, \ } @@ -104,7 +104,7 @@ struct test_data_de { #define define_test_de_attr(name, input, exp) \ do { \ static struct test_data_de data; \ - data.input_data = input; \ + data.input_data = g_memdup(input, sizeof (input)); \ data.input_size = sizeof(input); \ data.expected = exp; \ tester_add("/sdp/DE/ATTR/" name, &data, NULL, \ thanks for help. But similar happens in following failing tests with -fsanitize=address: FAIL: unit/test-avdtp FAIL: unit/test-avctp FAIL: unit/test-avrcp FAIL: unit/test-hfp FAIL: unit/test-gatt FAIL: unit/test-hog can you please prepare a patch where you'll factor out macros like #define raw_pdu(args...) into a header file? Thanks. (In reply to Martin Liška from comment #8) > But similar happens in following failing tests with -fsanitize=address: > > FAIL: unit/test-avdtp > FAIL: unit/test-avctp > FAIL: unit/test-avrcp > FAIL: unit/test-hfp > FAIL: unit/test-gatt > FAIL: unit/test-hog > > can you please prepare a patch where you'll factor out macros like > #define raw_pdu(args...) into a header file? > > Thanks. Im fixing that, though it is not possible to have it in a common header because the PDUs are different in each case. Btw, does define_test_de_attr really needs to be changed? There is no intermediate variable like in define_test so the variable should not go out of scope since it is static. Yes, it's problematic, please see explanation: https://gcc.gnu.org/ml/gcc-patches/2019-01/msg00629.html and there's a reduced example: $ cat /tmp/x.c struct test_data_de { const void *input_data; int input_size; }; int main() { struct test_data_de *ptr; { static struct test_data_de data; data.input_size = sizeof((const unsigned char[]) { 0x25, 0x00 }); data.input_data = ((const unsigned char[]) { 0x25, 0x00 }); ptr = &data; } *(char*)ptr->input_data = 'x'; return 0; } $ gcc /tmp/x.c -fsanitize=address && ./a.out ================================================================= ==17535==ERROR: AddressSanitizer: stack-use-after-scope on address 0x7fffffffdc20 at pc 0x0000004012d9 bp 0x7fffffffdbe0 sp 0x7fffffffdbd8 WRITE of size 1 at 0x7fffffffdc20 thread T0 #0 0x4012d8 in main (/home/marxin/Programming/bluez/a.out+0x4012d8) #1 0x7ffff7018fea in __libc_start_main ../csu/libc-start.c:308 #2 0x4010c9 in _start (/home/marxin/Programming/bluez/a.out+0x4010c9) Address 0x7fffffffdc20 is located in stack of thread T0 at offset 32 in frame #0 0x401181 in main (/home/marxin/Programming/bluez/a.out+0x401181) This frame has 1 object(s): [32, 34) '<unknown>' <== Memory access at offset 32 is inside this variable HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork (longjmp and C++ exceptions *are* supported) SUMMARY: AddressSanitizer: stack-use-after-scope (/home/marxin/Programming/bluez/a.out+0x4012d8) in main Shadow bytes around the buggy address: 0x10007fff7b30: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x10007fff7b40: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x10007fff7b50: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x10007fff7b60: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x10007fff7b70: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 =>0x10007fff7b80: f1 f1 f1 f1[f8]f3 f3 f3 00 00 00 00 00 00 00 00 0x10007fff7b90: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x10007fff7ba0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x10007fff7bb0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x10007fff7bc0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x10007fff7bd0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 Ive sent the patches to the mailing list. |