De: Pablo Montes Lozano <pmontes@xxxxxxxxxxxxxxxxx> Enviado: miércoles, 5 de febrero de 2025 9:57 Para: Luiz Augusto von Dentz <luiz.dentz@xxxxxxxxx> Cc: linux-bluetooth@xxxxxxxxxxxxxxx <linux-bluetooth@xxxxxxxxxxxxxxx> Asunto: RE: [PATCH BlueZ 1/2] emulator: Fix Werror=stringop-overflow Hi Luiz, I use uname -a Linux PCMALAGA-UB 6.8.0-52-generic #53-Ubuntu SMP PREEMPT_DYNAMIC Sat Jan 11 00:06:25 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux make --version GNU Make 4.3 Built for x86_64-pc-linux-gnu Copyright (C) 1988-2020 Free Software Foundation, Inc. Licence GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html> This is free software: you are free to change and redistribute it. There is NO WARRANTY, to the extent permitted by law. Full build steps: ============ After patch -------------- sudo apt build-dep bluez sudo apt install libsbc-dev libspeexdsp-dev <git clone and pull latest changes> cd bluez ./bootstrap-configure make distclean mkdir -p ../build && cd ../build ../bluez/configure --prefix=/home/bluez/usr --mandir=/home/bluez/man --sysconfdir=/home/bluez/etc \ --localstatedir=/home/bluez/var --enable-experimental --enable-mesh \ --enable-testing --enable-tools --enable-logger make make check Using latest changes make ends successfully. After the latest patches the warning does not appear to me. Checking out previous commit -------------------------------------- cd ../bluez git checkout e77884accdb22268eb65374fc96c35d9f8788d32 cd ../build-bluez && rm -rf * ../bluez/configure --prefix=/home/bluez/usr --mandir=/home/bluez/man --sysconfdir=/home/bluez/etc \ --localstatedir=/home/bluez/var --enable-experimental --enable-mesh \ --enable-testing --enable-tools --enable-logger make make check Make warns: CC emulator/serial.o In file included from /usr/include/features.h:502, from /usr/include/x86_64-linux-gnu/bits/libc-header-start.h:33, from /usr/include/stdio.h:28, from ../bluez/emulator/serial.c:17: In function ‘read’, inlined from ‘serial_read_callback’ at ../bluez/emulator/serial.c:90:8: /usr/include/x86_64-linux-gnu/bits/unistd.h:28:10: warning: ‘__read_alias’ specified size between 18446744073709490177 and 18446744073709551615 exceeds maximum object size 9223372036854775807 [-Wstringop-overflow=] 28 | return __glibc_fortify (read, __nbytes, sizeof (char), | ^~~~~~~~~~~~~~~ ../bluez/emulator/serial.c: In function ‘serial_read_callback’: ../bluez/emulator/serial.c:78:24: note: destination object allocated here 78 | static uint8_t buf[4096]; | ^~~ /usr/include/x86_64-linux-gnu/bits/unistd-decl.h:29:16: note: in a call to function ‘__read_alias’ declared with attribute ‘access (write_only, 2, 3)’ 29 | extern ssize_t __REDIRECT_FORTIFY (__read_alias, (int __fd, void *__buf, Maintainer build (previous commit) -------------------------------------------- Managed to reproduce the error cd ../build && make distclean && rm -rf * ../bluez/configure --prefix=/home/bluez/usr --mandir=/home/bluez/man --sysconfdir=/home/bluez/etc \ --localstatedir=/home/bluez/var --enable-experimental --enable-mesh \ --enable-testing --enable-tools --enable-logger --enable-maintainer-mode make CC emulator/serial.o In file included from /usr/include/features.h:502, from /usr/include/x86_64-linux-gnu/bits/libc-header-start.h:33, from /usr/include/stdio.h:28, from ../bluez/emulator/serial.c:17: In function ‘read’, inlined from ‘serial_read_callback’ at ../bluez/emulator/serial.c:90:8: /usr/include/x86_64-linux-gnu/bits/unistd.h:28:10: error: ‘__read_alias’ specified size between 18446744073709490177 and 18446744073709551615 exceeds maximum object size 9223372036854775807 [-Werror=stringop-overflow=] 28 | return __glibc_fortify (read, __nbytes, sizeof (char), | ^~~~~~~~~~~~~~~ ../bluez/emulator/serial.c: In function ‘serial_read_callback’: ../bluez/emulator/serial.c:78:24: note: destination object allocated here 78 | static uint8_t buf[4096]; | ^~~ /usr/include/x86_64-linux-gnu/bits/unistd-decl.h:29:16: note: in a call to function ‘__read_alias’ declared with attribute ‘access (write_only, 2, 3)’ 29 | extern ssize_t __REDIRECT_FORTIFY (__read_alias, (int __fd, void *__buf, | ^~~~~~~~~~~~~~~~~~ cc1: all warnings being treated as errors Tests ------- A unit test fails in any case, but I don't think it is related to this issue PASS: unit/test-bass CC unit/test-vcp.o CCLD unit/test-vcp ../bluez/test-driver: line 112: 36129 Aborted (core dumped) "$@" >> "$log_file" 2>&1 FAIL: unit/test-vcp CC unit/test_mesh_crypto-test-mesh-crypto.o CCLD unit/test-mesh-crypto PASS: unit/test-mesh-crypto ============================================================================ Testsuite summary for bluez 5.79 ============================================================================ # TOTAL: 31 # PASS: 30 # SKIP: 0 # XFAIL: 0 # FAIL: 1 # XPASS: 0 # ERROR: 0 ============================================================================ See ./test-suite.log ============================================================================ cat ./test-suite.log AICS/SR/SGGIT/CP/BI-01-C - init AICS/SR/SGGIT/CP/BI-01-C - setup AICS/SR/SGGIT/CP/BI-01-C - setup complete AICS/SR/SGGIT/CP/BI-01-C - run AICS/SR/SGGIT/CP/BI-01-C - test passed AICS/SR/SGGIT/CP/BI-01-C - teardown AICS/SR/SGGIT/CP/BI-01-C - teardown complete AICS/SR/SGGIT/CP/BI-01-C - done AICS/SR/SGGIT/CP/BI-02-C - init AICS/SR/SGGIT/CP/BI-02-C - setup AICS/SR/SGGIT/CP/BI-02-C - setup complete AICS/SR/SGGIT/CP/BI-02-C - run AICS/SR/SGGIT/CP/BI** ERROR:../bluez/src/shared/tester.c:981:test_io_recv: assertion failed (len == iov->iov_len): (5 == 6) -02-C - test passed AICS/SR/SGGIT/CP/BI-02-C - teardown AICS/SR/SGGIT/CP/BI-02-C - teardown complete AICS/SR/SGGIT/CP/BI-02-C - done AICS/SR/CP/BV-01-C - init AICS/SR/CP/BV-01-C - setup AICS/SR/CP/BV-01-C - setup complete AICS/SR/CP/BV-01-C - run gatt_notify_cb: Failed to send notification Bail out! ERROR:../bluez/src/shared/tester.c:981:test_io_recv: assertion failed (len == iov->iov_len): (5 == 6) FAIL unit/test-vcp (exit status: 134) ________________________________________ De: Luiz Augusto von Dentz <luiz.dentz@xxxxxxxxx> Enviado: martes, 4 de febrero de 2025 17:38 Para: Pablo Montes Lozano <pmontes@xxxxxxxxxxxxxxxxx> Cc: linux-bluetooth@xxxxxxxxxxxxxxx <linux-bluetooth@xxxxxxxxxxxxxxx> Asunto: Re: [PATCH BlueZ 1/2] emulator: Fix Werror=stringop-overflow Hi Pablo, On Tue, Feb 4, 2025 at 10:59 AM Luiz Augusto von Dentz <luiz.dentz@xxxxxxxxx> wrote: > > Hi Pablo, > > On Tue, Feb 4, 2025 at 5:17 AM Pablo Montes <pmontes@xxxxxxxxxxxxxxxxx> wrote: > > > > Warning on read for a possible packet offset > > greater than buffer size is treated as error. > > > > I suggest using ssize_t so it is always positive. > > Returning if packet offset makes no sense might > > not discard the whole packet and start again > > > > --- > > emulator/serial.c | 11 ++++++++++- > > 1 file changed, 10 insertions(+), 1 deletion(-) > > > > diff --git a/emulator/serial.c b/emulator/serial.c > > index b74556b13..13b844033 100644 > > --- a/emulator/serial.c > > +++ b/emulator/serial.c > > @@ -79,6 +79,7 @@ static void serial_read_callback(int fd, uint32_t events, void *user_data) > > uint8_t *ptr = buf; > > ssize_t len; > > uint16_t count; > > + ssize_t available; > > > > if (events & (EPOLLERR | EPOLLHUP)) { > > mainloop_remove_fd(serial->fd); > > @@ -87,8 +88,16 @@ static void serial_read_callback(int fd, uint32_t events, void *user_data) > > } > > > > again: > > + > > + if(serial->pkt_offset > sizeof(buf)) { > > + printf("packet offset overflow\n"); > > + serial->pkt_offset = 0; > > + return; > > + } > > + > > + available = sizeof(buf) - serial->pkt_offset; > > len = read(serial->fd, buf + serial->pkt_offset, > > - sizeof(buf) - serial->pkt_offset); > > + available); > > if (len < 0) { > > if (errno == EAGAIN) > > goto again; > > -- > > 2.43.0 > > I suspect the whole idea of buf being static is not really necessary > since pkt_data exists we can probably remove the static from buf: > > diff --git a/emulator/serial.c b/emulator/serial.c > index b74556b13547..f8062ae5eac3 100644 > --- a/emulator/serial.c > +++ b/emulator/serial.c > @@ -75,7 +75,7 @@ static void serial_write_callback(const struct iovec > *iov, int iovlen, > static void serial_read_callback(int fd, uint32_t events, void *user_data) > { > struct serial *serial = user_data; > - static uint8_t buf[4096]; > + uint8_t buf[4096]; > uint8_t *ptr = buf; > ssize_t len; > uint16_t count; > @@ -87,8 +87,7 @@ static void serial_read_callback(int fd, uint32_t > events, void *user_data) > } > > again: > - len = read(serial->fd, buf + serial->pkt_offset, > - sizeof(buf) - serial->pkt_offset); > + len = read(serial->fd, buf, sizeof(buf)); > if (len < 0) { > if (errno == EAGAIN) > goto again; > @@ -98,7 +97,7 @@ again: > if (!serial->btdev) > return; > > - count = serial->pkt_offset + len; > + count = len; > > while (count > 0) { > hci_command_hdr *cmd_hdr; > > > -- > Luiz Augusto von Dentz I'm trying to reproduce this, but even with -Wstringop-overflow it doesn't catch this case, perhaps I need -D_FORTIFY_SOURCE as well? Or maybe there is a way to detect what ubuntu is using so we can start including this as well. -- Luiz Augusto von Dentz
