| Message ID | 20250204101612.66823-1-pmontes@shsconsultores.es |
|---|---|
| Headers | show |
| Series | Fix Ubuntu 24.04 build error | expand |
Hi Pablo, On Tue, Feb 4, 2025 at 5:17 AM Pablo Montes <pmontes@shsconsultores.es> 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;
Hello: This series was applied to bluetooth/bluez.git (master) by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>: On Tue, 4 Feb 2025 11:16:10 +0100 you wrote: > On Ubuntu 24.04: build warnings for buffer read overflow are treated > as errors on emulator. This may be ignored with > CFLAGS="-Wno-error=stringop-overflow" but discarding packet if offset > is greater than buffer size removes the warning. > Could close GitHub issue #1049. > > Also I lost some time using mesh-cfgclient on a fresh Linux with no > .config folder in home: mesh-cfgclient failed but did not explain the > error, so printing a message could be welcome for others. > > [...] Here is the summary with links: - [BlueZ,1/2] emulator: Fix Werror=stringop-overflow (no matching commit) - [BlueZ,2/2] tools: print error on mkdir https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=9dbc92a360b2 You are awesome, thank you!
De: Pablo Montes Lozano <pmontes@shsconsultores.es> Enviado: miércoles, 5 de febrero de 2025 9:57 Para: Luiz Augusto von Dentz <luiz.dentz@gmail.com> Cc: linux-bluetooth@vger.kernel.org <linux-bluetooth@vger.kernel.org> 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)