Skip to content

Conversation

@aescolar
Copy link
Member

@aescolar aescolar commented Aug 9, 2024

gcc 13 produces a build warning (see below), as it seems to believe the number of read bytes may overflow the frame_size type.
Let's increase the frame_size bitwidth to avoid this. (Any 32bit type, signed or unsigned, avoids this warning)

The build warning:

In file included from /usr/include/features.h:502,
                 from /usr/include/i386-linux-gnu/bits/libc-header-start.h:33,
                 from /usr/include/stdint.h:26,
                 from /usr/lib/gcc/x86_64-linux-gnu/13/include/stdint.h:9,
                 from zephyr/include/zephyr/types.h:11,
                 from zephyr/include/zephyr/kernel_includes.h:21,
                 from zephyr/include/zephyr/kernel.h:17,
                 from zephyr/drivers/bluetooth/hci/userchan.c:9:
In function ‘read’,
    inlined from ‘rx_thread’ at drivers/bluetooth/hci/userchan.c:201:9:
/usr/include/i386-linux-gnu/bits/unistd.h:28:10: warning: ‘__read_alias’ specified size
between 4294902273 and 4294967295 exceeds maximum object size 2147483647
[-Wstringop-overflow=]
   28 |   return __glibc_fortify (read, __nbytes, sizeof (char),
      |          ^~~~~~~~~~~~~~~
drivers/bluetooth/hci/userchan.c: In function ‘rx_thread’:
drivers/bluetooth/hci/userchan.c:187:32: note: destination object allocated here
  187 |                 static uint8_t frame[512];
      |                                ^~~~~
/usr/include/i386-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,
      |                ^~~~~~~~~~~~~~~~~~

Fixes #76912

@aescolar aescolar added the Trivial Changes that can be reviewed by anyone, i.e. doc changes, minor build system tweaks, etc. label Aug 9, 2024
@aescolar aescolar requested a review from jori-nordic August 9, 2024 12:16
@zephyrbot zephyrbot added area: Bluetooth Host Bluetooth Host (excluding BR/EDR) area: Bluetooth size: XS A PR changing only a single line of code labels Aug 9, 2024
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't ssize_t be a better type, considering the usage (e.g. doing operations together with other ssize_t variables like len)?

That said, there's int32_t decoded_len later in the function which might be better updated to ssize_t too.

Copy link
Member Author

@aescolar aescolar Aug 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed :)
(decoded_len is already the return type of hci_packet_complete() so it should not be a problem even in theory :) )

gcc 13 produces a build warning (see below), as it seems to
believe the number of read bytes may overflow the frame_size
type.
Let's increase the frame_size bitwidth to avoid this.
(Any 32bit type, signed or unsigned, avoids this warning)

The build warning:
```
In file included from /usr/include/features.h:502,
                 from bits/libc-header-start.h:33,
                 from /usr/include/stdint.h:26,
                 from include/stdint.h:9,
                 from zephyr/include/zephyr/types.h:11,
                 from zephyr/include/zephyr/kernel_includes.h:21,
                 from zephyr/include/zephyr/kernel.h:17,
                 from zephyr/drivers/bluetooth/hci/userchan.c:9:
In function ‘read’,
    inlined from ‘rx_thread’ at drivers/bluetooth/hci/userchan.c:201:9:
/usr/include/i386-linux-gnu/bits/unistd.h:28:10: warning: ‘__read_alias’
specified size between 4294902273 and 4294967295 exceeds maximum object
size 2147483647
[-Wstringop-overflow=]
   28 |   return __glibc_fortify (read, __nbytes, sizeof (char),
      |          ^~~~~~~~~~~~~~~
drivers/bluetooth/hci/userchan.c: In function ‘rx_thread’:
drivers/bluetooth/hci/userchan.c:187:32: note: destination object allocated
  here
  187 |                 static uint8_t frame[512];
      |                                ^~~~~
/usr/include/i386-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
      |                ^~~~~~~~~~~~~~~~~~
```

Signed-off-by: Alberto Escolar Piedras <alberto.escolar.piedras@nordicsemi.no>
@aescolar aescolar added the backport v3.7-branch Request backport to the v3.7-branch label Aug 11, 2024
@carlescufi carlescufi merged commit ee08327 into zephyrproject-rtos:main Aug 12, 2024
@aescolar aescolar deleted the userchan_ovf branch August 12, 2024 11:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: Bluetooth Host Bluetooth Host (excluding BR/EDR) area: Bluetooth backport v3.7-branch Request backport to the v3.7-branch size: XS A PR changing only a single line of code Trivial Changes that can be reviewed by anyone, i.e. doc changes, minor build system tweaks, etc.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

drivers/bluetooth/hci/userchan.c build waring w gcc13

5 participants