From fd7956e57851ff7938375a7bfa2fec89324aaae2 Mon Sep 17 00:00:00 2001 From: Tianyi Gao Date: Fri, 8 May 2026 16:21:03 -0700 Subject: [PATCH] Fix double handoff for unloaded small messages --- homa_incoming.c | 20 ++++++++++---------- test/unit_homa_incoming.c | 12 +++++++++--- 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/homa_incoming.c b/homa_incoming.c index ee7272cf..faddc4d0 100644 --- a/homa_incoming.c +++ b/homa_incoming.c @@ -668,16 +668,6 @@ void homa_data_pkt(struct sk_buff *skb, struct homa_rpc *rpc) tt_addr(rpc->peer->addr), ntohl(h->seg.offset), ntohl(h->message_length)); - if (h->ack.client_id) { - const struct in6_addr saddr = skb_canonical_ipv6_saddr(skb); - - homa_rpc_unlock(rpc); - homa_rpc_acked(rpc->hsk, &saddr, &h->ack); - homa_rpc_lock(rpc); - if (rpc->state == RPC_DEAD) - goto discard; - } - if (rpc->state != RPC_INCOMING && homa_is_client(rpc->id)) { if (unlikely(rpc->state != RPC_OUTGOING)) goto discard; @@ -728,6 +718,16 @@ void homa_data_pkt(struct sk_buff *skb, struct homa_rpc *rpc) homa_rpc_handoff(rpc); } + if (h->ack.client_id) { + const struct in6_addr saddr = skb_canonical_ipv6_saddr(skb); + + homa_rpc_unlock(rpc); + homa_rpc_acked(rpc->hsk, &saddr, &h->ack); + homa_rpc_lock(rpc); + if (rpc->state == RPC_DEAD) + return; + } + #ifndef __STRIP__ /* See strip.py */ if (ntohs(h->cutoff_version) != homa->cutoff_version) { /* The sender has out-of-date cutoffs. Note: we may need diff --git a/test/unit_homa_incoming.c b/test/unit_homa_incoming.c index a0e02236..81c0e421 100644 --- a/test/unit_homa_incoming.c +++ b/test/unit_homa_incoming.c @@ -1418,10 +1418,16 @@ TEST_F(homa_incoming, homa_data_pkt__handle_ack_rpc_now_dead) 1400, 0), srpc); homa_rpc_unlock(srpc); EXPECT_EQ(RPC_DEAD, srpc->state); + /* With the issue #77 fix the piggy-backed ACK is processed *after* + * homa_add_packet has queued the skb, so by the time we observe + * RPC_DEAD the packet has been added (bytes_remaining: 8600 - 1400) + * and the function returns instead of going through `goto discard` + * (kfree_skb on a queued skb would be use-after-free; the msgin + * destructor reaps it on RPC teardown). + */ EXPECT_SUBSTR("ack 1235; " - "homa_rpc_end invoked; " - "homa_data_pkt discarded packet", unit_log_get()); - EXPECT_EQ(8600, srpc->msgin.bytes_remaining); + "homa_rpc_end invoked", unit_log_get()); + EXPECT_EQ(7200, srpc->msgin.bytes_remaining); } TEST_F(homa_incoming, homa_data_pkt__wrong_client_rpc_state) {