From aafd0d13920f64bb44445a838d1d00fbcd017763 Mon Sep 17 00:00:00 2001 From: "randomizedcoder dave.seddon.ca@gmail.com" Date: Fri, 18 Sep 2026 10:36:43 -0700 Subject: [PATCH] harden(incoming): read DATA seg.offset as u32 in homa_copy_to_user homa_copy_to_user() reads the DATA segment offset into a signed int: int offset = ntohl(h->seg.offset); The offset is an unsigned wire field, so for consistency with homa_add_packet() (which already uses `u32 start = ntohl(h->seg.offset)`) read it as u32 here too. Note: this is a defense-in-depth / type-consistency change, not a live bug. Packets only reach homa_copy_to_user() after homa_add_packet() has validated the offset as u32 and dropped anything with start >= rpc->msgin.length; msgin.length is itself capped at HOMA_MAX_MESSAGE_LENGTH (1,000,000) by homa_message_in_init(). So offsets that reach the copy-out path are always small and non-negative -- the old `int` was safe in practice. start_offset/end_offset and offset+copied are unaffected (values stay well within int range). Adds a characterization test, homa_data_pkt__seg_offset_is_unsigned, covering the sign-boundary offset 0x80000000: it is dropped as PKT_TOO_BIG and never queued (green before and after -- it locks in the existing guarantee at the signed/unsigned boundary, complementing the existing offset-2000000 case). Incoming suite: 123/123. Co-Authored-By: Claude Opus 4.8 --- homa_incoming.c | 2 +- test/unit_homa_incoming.c | 24 ++++++++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/homa_incoming.c b/homa_incoming.c index 9e72c521..8ba5d633 100644 --- a/homa_incoming.c +++ b/homa_incoming.c @@ -395,7 +395,7 @@ int homa_copy_to_user(struct homa_rpc *rpc) struct homa_data_hdr *h = (struct homa_data_hdr *) skbs[i]->data; int pkt_length = homa_data_len(skbs[i]); - int offset = ntohl(h->seg.offset); + u32 offset = ntohl(h->seg.offset); int buf_bytes, chunk_size; struct iov_iter iter; int copied = 0; diff --git a/test/unit_homa_incoming.c b/test/unit_homa_incoming.c index 74b4b05e..566ea7b3 100644 --- a/test/unit_homa_incoming.c +++ b/test/unit_homa_incoming.c @@ -1754,6 +1754,30 @@ TEST_F(homa_incoming, homa_data_pkt__homa_add_packet_returns_error) EXPECT_EQ(1, mock_num_drop_reasons); EXPECT_EQ(SKB_DROP_REASON_PKT_TOO_BIG, mock_drop_reasons[0]); } +TEST_F(homa_incoming, homa_data_pkt__seg_offset_is_unsigned) +{ + /* A DATA seg.offset is an unsigned wire field. homa_add_packet() + * validates it as u32 before the packet is queued, so an offset with + * the top bit set is rejected as PKT_TOO_BIG (it is >= msgin.length, + * which is capped at HOMA_MAX_MESSAGE_LENGTH) and never reaches the + * copy-out path. This characterizes that boundary and guards the u32 + * handling in homa_copy_to_user's offset. + */ + struct homa_rpc *crpc = unit_client_rpc(&self->hsk, + UNIT_OUTGOING, self->client_ip, self->server_ip, + self->server_port, self->client_id, 1000, 1600); + + ASSERT_NE(NULL, crpc); + unit_log_clear(); + crpc->msgout.next_xmit_offset = crpc->msgout.length; + self->data.message_length = htonl(1600); + self->data.seg.offset = htonl(0x80000000); + homa_data_pkt(mock_skb_alloc(self->server_ip, self->client_ip, + &self->data.common, 1400, 0), crpc); + EXPECT_EQ(0, skb_queue_len(&crpc->msgin.packets)); + EXPECT_EQ(1, mock_num_drop_reasons); + EXPECT_EQ(SKB_DROP_REASON_PKT_TOO_BIG, mock_drop_reasons[0]); +} TEST_F(homa_incoming, homa_data_pkt__handoff) { struct homa_rpc *crpc = unit_client_rpc(&self->hsk,