[PATCH] tcp: Add flags frames to payload frames arrays
There is an issue reported by Volker Diels-Grabsch and Boleyn Su.
A segmentation fault occurs when executing the following command:
(sleep 0.1; ssh -p 22000 127.0.0.1) & passt -f -t 22000:22
It's caused by commit 78da088f7bab ("tcp: unify payload and flags
l2 frames array"). Fix it by adding flags frames to payload frames
arrays.
Signed-off-by: Yumei Huang
On Wed, 10 Sep 2025 15:46:30 +0800
Yumei Huang
There is an issue reported by Volker Diels-Grabsch and Boleyn Su. A segmentation fault occurs when executing the following command:
(sleep 0.1; ssh -p 22000 127.0.0.1) & passt -f -t 22000:22
It's caused by commit 78da088f7bab ("tcp: unify payload and flags l2 frames array"). Fix it by adding flags frames to payload frames arrays.
Ouch. Nice catch!
I guess in this sentence you meant "Fix it by setting ownership
information for flags frames"? The patch itself looks correct, the
commit message doesn't entirely match it though.
The commit title is also a bit mismatching. The description for
tcp_frame_conns[] is:
/* References tracking the owner connection of frames in the tap outqueue */
A couple of minor details you probably didn't know about:
- in this case we would typically use all these tags in the commit
message:
Reported-by: Volker Diels-Grabsch
Signed-off-by: Yumei Huang
--- tcp_buf.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tcp_buf.c b/tcp_buf.c index bc898de..d351c20 100644 --- a/tcp_buf.c +++ b/tcp_buf.c @@ -209,13 +209,14 @@ int tcp_buf_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags) if (ret <= 0) return ret;
- tcp_payload_used++; + tcp_frame_conns[tcp_payload_used++] = conn; l4len = optlen + sizeof(struct tcphdr); iov[TCP_IOV_PAYLOAD].iov_len = l4len; tcp_l2_buf_fill_headers(conn, iov, NULL, seq, false);
if (flags & DUP_ACK) { struct iovec *dup_iov = tcp_l2_iov[tcp_payload_used++]; + tcp_frame_conns[tcp_payload_used - 1] = conn;
memcpy(dup_iov[TCP_IOV_TAP].iov_base, iov[TCP_IOV_TAP].iov_base, iov[TCP_IOV_TAP].iov_len);
-- Stefano
Thank you Stefano for all the information.
I will send a v2 soon.
On Wed, Sep 10, 2025 at 4:52 PM Stefano Brivio
On Wed, 10 Sep 2025 15:46:30 +0800 Yumei Huang
wrote: There is an issue reported by Volker Diels-Grabsch and Boleyn Su. A segmentation fault occurs when executing the following command:
(sleep 0.1; ssh -p 22000 127.0.0.1) & passt -f -t 22000:22
It's caused by commit 78da088f7bab ("tcp: unify payload and flags l2 frames array"). Fix it by adding flags frames to payload frames arrays.
Ouch. Nice catch!
I guess in this sentence you meant "Fix it by setting ownership information for flags frames"? The patch itself looks correct, the commit message doesn't entirely match it though.
The commit title is also a bit mismatching. The description for tcp_frame_conns[] is:
/* References tracking the owner connection of frames in the tap outqueue */
A couple of minor details you probably didn't know about:
- in this case we would typically use all these tags in the commit message:
Reported-by: Volker Diels-Grabsch
Reported-by: Boleyn Su Fixes: 78da088f7bab ("tcp: unify payload and flags l2 frames array") - you should Cc: Jon as the author of the faulty commit (I just did)
- David's upstream email address is david@gibson.dropbear.id.au (I just changed it)
I can add the tags for you, but it's probably easier if you re-post a v2 with a more accurate commit message. Thanks!
Signed-off-by: Yumei Huang
--- tcp_buf.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tcp_buf.c b/tcp_buf.c index bc898de..d351c20 100644 --- a/tcp_buf.c +++ b/tcp_buf.c @@ -209,13 +209,14 @@ int tcp_buf_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags) if (ret <= 0) return ret;
- tcp_payload_used++; + tcp_frame_conns[tcp_payload_used++] = conn; l4len = optlen + sizeof(struct tcphdr); iov[TCP_IOV_PAYLOAD].iov_len = l4len; tcp_l2_buf_fill_headers(conn, iov, NULL, seq, false);
if (flags & DUP_ACK) { struct iovec *dup_iov = tcp_l2_iov[tcp_payload_used++]; + tcp_frame_conns[tcp_payload_used - 1] = conn;
memcpy(dup_iov[TCP_IOV_TAP].iov_base, iov[TCP_IOV_TAP].iov_base, iov[TCP_IOV_TAP].iov_len);
-- Stefano
-- Thanks, Yumei Huang
participants (2)
-
Stefano Brivio
-
Yumei Huang