From 3a72ba2aede63e49e53fe22b80e07c9d3c8f72e3 Mon Sep 17 00:00:00 2001 From: Amaury Denoyelle Date: Mon, 14 Nov 2022 11:41:34 +0100 Subject: [PATCH] BUILD: quic: fix dubious 0-byte overflow on qc_release_lost_pkts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With GCC 12.2.0 and O2 optimization activated, compiler reports the following warning for qc_release_lost_pkts(). In function ‘quic_tx_packet_refdec’, inlined from ‘qc_release_lost_pkts.constprop’ at src/quic_conn.c:2056:3: include/haproxy/atomic.h:320:41: error: ‘__atomic_sub_fetch_4’ writing 4 bytes into a region of size 0 overflows the destination [-Werror=stringop-overflow=] 320 | #define HA_ATOMIC_SUB_FETCH(val, i) __atomic_sub_fetch(val, i, __ATOMIC_SEQ_CST) | ^~~~~~~~~~~~~~~~~~ include/haproxy/quic_conn.h:499:14: note: in expansion of macro ‘HA_ATOMIC_SUB_FETCH’ 499 | if (!HA_ATOMIC_SUB_FETCH(&pkt->refcnt, 1)) { | ^~~~~~~~~~~~~~~~~~~ GCC thinks that quic_tx_packet_refdec() can be called with a NULL argument from qc_release_lost_pkts() with as arg. This warning is a false positive as cannot be NULL in qc_release_lost_pkts() at this stage. This is due to the previous check to ensure that list is not empty. This warning is silenced by using ALREADY_CHECKED() macro. This should be backported up to 2.6. This should fix github issue #1852. --- src/quic_conn.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/quic_conn.c b/src/quic_conn.c index a110e9c23d..8328862601 100644 --- a/src/quic_conn.c +++ b/src/quic_conn.c @@ -2052,6 +2052,11 @@ static inline void qc_release_lost_pkts(struct quic_conn *qc, qc->path->cc.algo->slow_start(&qc->path->cc); } + /* cannot be NULL at this stage because we have ensured + * that list is not empty. Without this, GCC 12.2.0 reports a + * possible overflow on a 0 byte region with O2 optimization. + */ + ALREADY_CHECKED(oldest_lost); quic_tx_packet_refdec(oldest_lost); if (newest_lost != oldest_lost) quic_tx_packet_refdec(newest_lost);