BUG/MINOR: quic: Retransmitted frames marked as acknowledged

Obviously, frames which are duplicated from others must not be retransmitted if
the original frame they were duplicated from was already acknowledged.
This should have been detected by qc_build_frms() which skips such frames,
except if the QUIC xprt does really bad things which are not supported by
the upper layer. This will have to be checked with Amaury.

To prevent the retransmision of these frames which leads to crashes as reported by
hpn0t0ad this gdb backtrace in GH #1809 where the frame builder tries to copy a huge
number of bytes to the packet buffer:

Thread 7 (Thread 0x7fddf373a700 (LWP 13)):
 #0  __memmove_sse2_unaligned_erms () at ../sysdeps/x86_64/multiarch/memmove-vec-unaligned-erms.S:520
No locals.
 #1  0x000055b17435705e in quic_build_stream_frame (buf=0x7fddf372ef78, end=<optimized out>, frm=0x7fdde08d3470, conn=<optimized out>) at src/quic_frame.c:515
        to_copy = 18446697703428890384
        stream = 0x7fdde08d3490
        wrap = <optimized out>

which matches this part of quic_frame.c code:

    wrap = (const unsigned char *)b_wrap(stream->buf);
    if (stream->data + stream->len > wrap) {
        size_t to_copy = wrap - stream->data;
        memcpy(*buf, stream->data, to_copy);
        *buf += to_copy;

we release as soon as possible the impacted frames as there is really no need
to retransmit such frames.

Thank you to @hpn0t0ad for having provided us with useful traces in github
issue #1809.

Must be backported in 2.6.

diff --git a/src/xprt_quic.c b/src/xprt_quic.c
index 3495d4d8b052c7495fe79f0fe031764bafec602f..d8c0f5e9471acee920db9beca29a7b58a102cf79 100644 (file)
--- a/src/xprt_quic.c
+++ b/src/xprt_quic.c
@@ -1515,17 +1515,20 @@ void qc_release_frm(struct quic_conn *qc, struct quic_frame *frm)
         */
        list_for_each_entry_safe(f, tmp, &origin->reflist, ref) {
                if (f->pkt) {
+                       f->flags |= QUIC_FL_TX_FRAME_ACKED;
+                       f->origin = NULL;
+                       LIST_DELETE(&f->ref);
                        pn = f->pkt->pn_node.key;
                        TRACE_DEVEL("mark frame as acked from packet",
                                    QUIC_EV_CONN_PRSAFRM, qc, f, &pn);
                }
                else {
-                       TRACE_DEVEL("mark unsent frame as acked",
+                       TRACE_DEVEL("freeing unsent frame",
                                    QUIC_EV_CONN_PRSAFRM, qc, f);
+                       LIST_DELETE(&f->ref);
+                       LIST_DELETE(&f->list);
+                       pool_free(pool_head_quic_frame, f);
                }
-               f->flags |= QUIC_FL_TX_FRAME_ACKED;
-               f->origin = NULL;
-               LIST_DELETE(&f->ref);
        }
        LIST_DELETE(&frm->list);
        pn = frm->pkt->pn_node.key;