[PATCH] hw/net/rtl8319: Work around GCC sanitizer / -Wstringop-overflow bug

Peter Maydell posted 1 patch 1 month, 1 week ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260305111832.1068093-1-peter.maydell@linaro.org
Maintainers: Jason Wang <jasowang@redhat.com>
There is a newer version of this series
hw/net/rtl8139.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
[PATCH] hw/net/rtl8319: Work around GCC sanitizer / -Wstringop-overflow bug
Posted by Peter Maydell 1 month, 1 week ago
If you compile QEMU with GCC with -fsanitize=address and
-Wstringop-overflow, this causes GCC to produce a false-positive
warning which it does not produce when the sanitizer is not enabled
(and which makes compilation fail if you're using -Werror, as we do
by default for builds from git):

../../hw/net/rtl8139.c: In function ‘rtl8139_io_writeb’:
../../hw/net/rtl8139.c:2264:17: error: writing 8 bytes into a region of size 0 [-Werror=stringop-overflow=]
 2264 |                 memcpy(data_to_checksum, saved_ip_header + 12, 8);
      |                 ^
In file included from ../../hw/net/rtl8139.c:62:
/home/pm215/qemu/include/net/eth.h:50:14: note: at offset [8, 48] into destination object ‘ip_ver_len’ of size 1
   50 |     uint8_t  ip_ver_len;     /* version and header length */
      |              ^~~~~~~~~~
../../hw/net/rtl8139.c:2192:21: error: writing 8 bytes into a region of size 0 [-Werror=stringop-overflow=]
 2192 |                     memcpy(data_to_checksum, saved_ip_header + 12, 8);
      |                     ^
/home/pm215/qemu/include/net/eth.h:50:14: note: at offset [8, 48] into destination object ‘ip_ver_len’ of size 1
   50 |     uint8_t  ip_ver_len;     /* version and header length */
      |              ^~~~~~~~~~
../../hw/net/rtl8139.c:2192:21: error: writing 8 bytes into a region of size 0 [-Werror=stringop-overflow=]
 2192 |                     memcpy(data_to_checksum, saved_ip_header + 12, 8);
      |                     ^
/home/pm215/qemu/include/net/eth.h:50:14: note: at offset [8, 48] into destination object ‘ip_ver_len’ of size 1
   50 |     uint8_t  ip_ver_len;     /* version and header length */
      |              ^~~~~~~~~~
In file included from /home/pm215/qemu/include/system/memory.h:21,
                 from /home/pm215/qemu/include/hw/pci/pci.h:4,
                 from /home/pm215/qemu/include/hw/pci/pci_device.h:4,
                 from ../../hw/net/rtl8139.c:54:
In function ‘stl_he_p’,
    inlined from ‘stl_be_p’ at /home/pm215/qemu/include/qemu/bswap.h:371:5,
    inlined from ‘rtl8139_cplus_transmit_one’ at ../../hw/net/rtl8139.c:2244:21,
    inlined from ‘rtl8139_cplus_transmit’ at ../../hw/net/rtl8139.c:2345:28,
    inlined from ‘rtl8139_io_writeb’ at ../../hw/net/rtl8139.c:2728:17:
/home/pm215/qemu/include/qemu/bswap.h:284:5: error: writing 4 bytes into a region of size 0 [-Werror=stringop-overflow=]
  284 |     __builtin_memcpy(ptr, &v, sizeof(v));
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/pm215/qemu/include/net/eth.h: In function ‘rtl8139_io_writeb’:
/home/pm215/qemu/include/net/eth.h:50:14: note: at offset [24, 64] into destination object ‘ip_ver_len’ of size 1
   50 |     uint8_t  ip_ver_len;     /* version and header length */
      |              ^~~~~~~~~~

This has been triaged as a bug in GCC:
 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114494
 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99673
(the sanitizer pass rewrites the IR in a way that conflicts with its
use by the warning pass that runs afterwards).

Since this is the only place in our code where we hit this, work
around it by rewriting the relevant bit of code, and noting in a
comment why we do so.

Cc: qemu-stable@nongnu.org
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3006
Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
Here's a patch that takes the "just work around this one problem"
approach to the gcc sanitizer compile failure.

On the fence about whether this is worth backporting to stable.
---
 hw/net/rtl8139.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c
index 2ad6338ebe..eb4dfcfbe0 100644
--- a/hw/net/rtl8139.c
+++ b/hw/net/rtl8139.c
@@ -2092,7 +2092,20 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s)
             eth_payload_data = saved_buffer + ETH_HLEN;
             eth_payload_len  = saved_size   - ETH_HLEN;
 
-            ip = (struct ip_header*)eth_payload_data;
+            /*
+             * It would be more natural to write this as
+             *   ip = (struct ip_header *)eth_payload_data;
+             * (the IP header is at the start of the ethernet payload).
+             * However, writing it that way triggers a GCC bug where an
+             * interaction between -fsanitize=address and -Wstringop-overflow
+             * results in a false-positive stringop-overflow warning that is
+             * only emitted when the address sanitizer is enabled:
+             *     https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114494
+             *     https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99673
+             * So we work around this by writing the expression in an equivalent
+             * way that doesn't run into this bug.
+             */
+            ip = (struct ip_header *)saved_buffer + ETH_HLEN;
 
             if (IP_HEADER_VERSION(ip) != IP_HEADER_VERSION_4) {
                 DPRINTF("+++ C+ mode packet has bad IP version %d "
-- 
2.43.0


Re: [PATCH] hw/net/rtl8319: Work around GCC sanitizer / -Wstringop-overflow bug
Posted by Thomas Huth 1 month, 1 week ago
On 05/03/2026 12.18, Peter Maydell wrote:
> If you compile QEMU with GCC with -fsanitize=address and
> -Wstringop-overflow, this causes GCC to produce a false-positive
> warning which it does not produce when the sanitizer is not enabled
> (and which makes compilation fail if you're using -Werror, as we do
> by default for builds from git):
> 
> ../../hw/net/rtl8139.c: In function ‘rtl8139_io_writeb’:
> ../../hw/net/rtl8139.c:2264:17: error: writing 8 bytes into a region of size 0 [-Werror=stringop-overflow=]
>   2264 |                 memcpy(data_to_checksum, saved_ip_header + 12, 8);
>        |                 ^
> In file included from ../../hw/net/rtl8139.c:62:
> /home/pm215/qemu/include/net/eth.h:50:14: note: at offset [8, 48] into destination object ‘ip_ver_len’ of size 1
>     50 |     uint8_t  ip_ver_len;     /* version and header length */
>        |              ^~~~~~~~~~
> ../../hw/net/rtl8139.c:2192:21: error: writing 8 bytes into a region of size 0 [-Werror=stringop-overflow=]
>   2192 |                     memcpy(data_to_checksum, saved_ip_header + 12, 8);
>        |                     ^
> /home/pm215/qemu/include/net/eth.h:50:14: note: at offset [8, 48] into destination object ‘ip_ver_len’ of size 1
>     50 |     uint8_t  ip_ver_len;     /* version and header length */
>        |              ^~~~~~~~~~
> ../../hw/net/rtl8139.c:2192:21: error: writing 8 bytes into a region of size 0 [-Werror=stringop-overflow=]
>   2192 |                     memcpy(data_to_checksum, saved_ip_header + 12, 8);
>        |                     ^
> /home/pm215/qemu/include/net/eth.h:50:14: note: at offset [8, 48] into destination object ‘ip_ver_len’ of size 1
>     50 |     uint8_t  ip_ver_len;     /* version and header length */
>        |              ^~~~~~~~~~
> In file included from /home/pm215/qemu/include/system/memory.h:21,
>                   from /home/pm215/qemu/include/hw/pci/pci.h:4,
>                   from /home/pm215/qemu/include/hw/pci/pci_device.h:4,
>                   from ../../hw/net/rtl8139.c:54:
> In function ‘stl_he_p’,
>      inlined from ‘stl_be_p’ at /home/pm215/qemu/include/qemu/bswap.h:371:5,
>      inlined from ‘rtl8139_cplus_transmit_one’ at ../../hw/net/rtl8139.c:2244:21,
>      inlined from ‘rtl8139_cplus_transmit’ at ../../hw/net/rtl8139.c:2345:28,
>      inlined from ‘rtl8139_io_writeb’ at ../../hw/net/rtl8139.c:2728:17:
> /home/pm215/qemu/include/qemu/bswap.h:284:5: error: writing 4 bytes into a region of size 0 [-Werror=stringop-overflow=]
>    284 |     __builtin_memcpy(ptr, &v, sizeof(v));
>        |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> /home/pm215/qemu/include/net/eth.h: In function ‘rtl8139_io_writeb’:
> /home/pm215/qemu/include/net/eth.h:50:14: note: at offset [24, 64] into destination object ‘ip_ver_len’ of size 1
>     50 |     uint8_t  ip_ver_len;     /* version and header length */
>        |              ^~~~~~~~~~
> 
> This has been triaged as a bug in GCC:
>   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114494
>   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99673
> (the sanitizer pass rewrites the IR in a way that conflicts with its
> use by the warning pass that runs afterwards).
> 
> Since this is the only place in our code where we hit this, work
> around it by rewriting the relevant bit of code, and noting in a
> comment why we do so.
> 
> Cc: qemu-stable@nongnu.org
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3006
> Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> Here's a patch that takes the "just work around this one problem"
> approach to the gcc sanitizer compile failure.
> 
> On the fence about whether this is worth backporting to stable.
> ---
>   hw/net/rtl8139.c | 15 ++++++++++++++-
>   1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c
> index 2ad6338ebe..eb4dfcfbe0 100644
> --- a/hw/net/rtl8139.c
> +++ b/hw/net/rtl8139.c
> @@ -2092,7 +2092,20 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s)
>               eth_payload_data = saved_buffer + ETH_HLEN;
>               eth_payload_len  = saved_size   - ETH_HLEN;
>   
> -            ip = (struct ip_header*)eth_payload_data;
> +            /*
> +             * It would be more natural to write this as
> +             *   ip = (struct ip_header *)eth_payload_data;
> +             * (the IP header is at the start of the ethernet payload).
> +             * However, writing it that way triggers a GCC bug where an
> +             * interaction between -fsanitize=address and -Wstringop-overflow
> +             * results in a false-positive stringop-overflow warning that is
> +             * only emitted when the address sanitizer is enabled:
> +             *     https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114494
> +             *     https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99673
> +             * So we work around this by writing the expression in an equivalent
> +             * way that doesn't run into this bug.
> +             */
> +            ip = (struct ip_header *)saved_buffer + ETH_HLEN;

Uh, isn't that a different pointer location? I mean, this first casts to 
struct ip_header*, then does the math on that pointer type, i.e. it adds 
ETH_HLEN * sizeof(struct ip_header) bytes?

I think you need this instead:

         ip = (struct ip_header *)(saved_buffer + ETH_HLEN);

But is the warning still gone in that case?

  Thomas


Re: [PATCH] hw/net/rtl8319: Work around GCC sanitizer / -Wstringop-overflow bug
Posted by Peter Maydell 1 month, 1 week ago
On Thu, 5 Mar 2026 at 12:18, Thomas Huth <thuth@redhat.com> wrote:
>
> On 05/03/2026 12.18, Peter Maydell wrote:



> > -            ip = (struct ip_header*)eth_payload_data;
> > +            /*
> > +             * It would be more natural to write this as
> > +             *   ip = (struct ip_header *)eth_payload_data;
> > +             * (the IP header is at the start of the ethernet payload).
> > +             * However, writing it that way triggers a GCC bug where an
> > +             * interaction between -fsanitize=address and -Wstringop-overflow
> > +             * results in a false-positive stringop-overflow warning that is
> > +             * only emitted when the address sanitizer is enabled:
> > +             *     https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114494
> > +             *     https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99673
> > +             * So we work around this by writing the expression in an equivalent
> > +             * way that doesn't run into this bug.
> > +             */
> > +            ip = (struct ip_header *)saved_buffer + ETH_HLEN;
>
> Uh, isn't that a different pointer location? I mean, this first casts to
> struct ip_header*, then does the math on that pointer type, i.e. it adds
> ETH_HLEN * sizeof(struct ip_header) bytes?
>
> I think you need this instead:
>
>          ip = (struct ip_header *)(saved_buffer + ETH_HLEN);
>
> But is the warning still gone in that case?

Rats, you're right -- good catch. And no, if we fix the expression
then the warning comes back. I guess we'll have to do it with
a warning-disabling pragma :-(

-- PMM
Re: [PATCH] hw/net/rtl8319: Work around GCC sanitizer / -Wstringop-overflow bug
Posted by Daniel P. Berrangé 1 month, 1 week ago
On Thu, Mar 05, 2026 at 11:18:32AM +0000, Peter Maydell wrote:
> If you compile QEMU with GCC with -fsanitize=address and
> -Wstringop-overflow, this causes GCC to produce a false-positive
> warning which it does not produce when the sanitizer is not enabled
> (and which makes compilation fail if you're using -Werror, as we do
> by default for builds from git):
> 
> ../../hw/net/rtl8139.c: In function ‘rtl8139_io_writeb’:
> ../../hw/net/rtl8139.c:2264:17: error: writing 8 bytes into a region of size 0 [-Werror=stringop-overflow=]
>  2264 |                 memcpy(data_to_checksum, saved_ip_header + 12, 8);
>       |                 ^
> In file included from ../../hw/net/rtl8139.c:62:
> /home/pm215/qemu/include/net/eth.h:50:14: note: at offset [8, 48] into destination object ‘ip_ver_len’ of size 1
>    50 |     uint8_t  ip_ver_len;     /* version and header length */
>       |              ^~~~~~~~~~
> ../../hw/net/rtl8139.c:2192:21: error: writing 8 bytes into a region of size 0 [-Werror=stringop-overflow=]
>  2192 |                     memcpy(data_to_checksum, saved_ip_header + 12, 8);
>       |                     ^
> /home/pm215/qemu/include/net/eth.h:50:14: note: at offset [8, 48] into destination object ‘ip_ver_len’ of size 1
>    50 |     uint8_t  ip_ver_len;     /* version and header length */
>       |              ^~~~~~~~~~
> ../../hw/net/rtl8139.c:2192:21: error: writing 8 bytes into a region of size 0 [-Werror=stringop-overflow=]
>  2192 |                     memcpy(data_to_checksum, saved_ip_header + 12, 8);
>       |                     ^
> /home/pm215/qemu/include/net/eth.h:50:14: note: at offset [8, 48] into destination object ‘ip_ver_len’ of size 1
>    50 |     uint8_t  ip_ver_len;     /* version and header length */
>       |              ^~~~~~~~~~
> In file included from /home/pm215/qemu/include/system/memory.h:21,
>                  from /home/pm215/qemu/include/hw/pci/pci.h:4,
>                  from /home/pm215/qemu/include/hw/pci/pci_device.h:4,
>                  from ../../hw/net/rtl8139.c:54:
> In function ‘stl_he_p’,
>     inlined from ‘stl_be_p’ at /home/pm215/qemu/include/qemu/bswap.h:371:5,
>     inlined from ‘rtl8139_cplus_transmit_one’ at ../../hw/net/rtl8139.c:2244:21,
>     inlined from ‘rtl8139_cplus_transmit’ at ../../hw/net/rtl8139.c:2345:28,
>     inlined from ‘rtl8139_io_writeb’ at ../../hw/net/rtl8139.c:2728:17:
> /home/pm215/qemu/include/qemu/bswap.h:284:5: error: writing 4 bytes into a region of size 0 [-Werror=stringop-overflow=]
>   284 |     __builtin_memcpy(ptr, &v, sizeof(v));
>       |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> /home/pm215/qemu/include/net/eth.h: In function ‘rtl8139_io_writeb’:
> /home/pm215/qemu/include/net/eth.h:50:14: note: at offset [24, 64] into destination object ‘ip_ver_len’ of size 1
>    50 |     uint8_t  ip_ver_len;     /* version and header length */
>       |              ^~~~~~~~~~
> 
> This has been triaged as a bug in GCC:
>  https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114494
>  https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99673
> (the sanitizer pass rewrites the IR in a way that conflicts with its
> use by the warning pass that runs afterwards).
> 
> Since this is the only place in our code where we hit this, work
> around it by rewriting the relevant bit of code, and noting in a
> comment why we do so.
> 
> Cc: qemu-stable@nongnu.org
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3006
> Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> Here's a patch that takes the "just work around this one problem"
> approach to the gcc sanitizer compile failure.

The alternative is a "#pragma push" to temp disable the
-Wstringop-overflow warning, though that would need to be
done in the 2 code locations triggering the bogus warning,
instead of this which does the single code location that
first confuses GCC, leading to the later warnings. I don't
mind either way though so...


> 
> On the fence about whether this is worth backporting to stable.
> ---
>  hw/net/rtl8139.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|


Re: [PATCH] hw/net/rtl8319: Work around GCC sanitizer / -Wstringop-overflow bug
Posted by Peter Maydell 1 month, 1 week ago
On Thu, 5 Mar 2026 at 11:33, Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Thu, Mar 05, 2026 at 11:18:32AM +0000, Peter Maydell wrote:
> > Here's a patch that takes the "just work around this one problem"
> > approach to the gcc sanitizer compile failure.
>
> The alternative is a "#pragma push" to temp disable the
> -Wstringop-overflow warning, though that would need to be
> done in the 2 code locations triggering the bogus warning,
> instead of this which does the single code location that
> first confuses GCC, leading to the later warnings. I don't
> mind either way though so...

I preferred not to do that, partly beacuse the warning
is triggered in three places, and partly because this
code's use of memcpy() is pretty confusing to me and I'd
rather we kept the warning enabled on those calls if we can,
so we don't accidentally silence any genuine overrun bugs here.

thanks
-- PMM