Re: [PATCH BlueZ 1/2] emulator: Fix Werror=stringop-overflow

[Luiz Augusto von Dentz <luiz.dentz@xxxxxxxxx>]

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