Skip to content

More fixes (fixes-4) #32

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
gasbytes wants to merge 8 commits into
base: master
Choose a base branch
from
+142 −10
Open

More fixes (fixes-4) #32

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
2a36ed7
initialize the oldest anacknowledged sequence number to the initial s…
gasbytes Feb 13, 2026
5d4b920
explicit wrap handling
gasbytes Feb 13, 2026
539a212
missing int cast after unsigned substraction
gasbytes Feb 17, 2026
d3fb63a
reject dduplicate syns on listener when an established connection with
gasbytes Feb 17, 2026
dcc97cf
wrap-aware substraction with tcp_seq_diff when seq < ack
gasbytes Feb 18, 2026
07db050
adding U suffix to fix unsigned operations problems when doing bitwise
gasbytes Feb 18, 2026
62f2b7b
regression tests
gasbytes Feb 18, 2026
f6d3e90
fixing wrapping substraction (using tcp_seq_diff)
gasbytes Feb 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
101 changes: 100 additions & 1 deletion src/test/unit/unit.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13853,7 +13853,100 @@ START_TEST(test_icmp_socket_send_recv)
}
END_TEST

START_TEST(test_regression_snd_una_initialized_on_syn_rcvd)
{
struct wolfIP s;
int sd;
struct wolfIP_sockaddr_in sin;
struct tsocket *ts;
ip4 local_ip = 0x0A000001U;
uint16_t local_port = 8080;

wolfIP_init(&s);
mock_link_init(&s);
wolfIP_ipconfig_set(&s, local_ip, 0xFFFFFF00U, 0);

sd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_STREAM, WI_IPPROTO_TCP);
ck_assert_int_gt(sd, 0);

memset(&sin, 0, sizeof(sin));
sin.sin_family = AF_INET;
sin.sin_port = ee16(local_port);
sin.sin_addr.s_addr = ee32(local_ip);
wolfIP_sock_bind(&s, sd, (struct wolfIP_sockaddr *)&sin, sizeof(sin));
wolfIP_sock_listen(&s, sd, 1);

/* inject_tcp_syn uses a deterministic PRNG seed so seq is reproducible */
inject_tcp_syn(&s, TEST_PRIMARY_IF, local_ip, local_port);

ts = &s.tcpsockets[SOCKET_UNMARK(sd)];
ck_assert_int_eq(ts->sock.tcp.state, TCP_SYN_RCVD);

/* snd_una must equal the ISN that was placed in seq, not 0 */
ck_assert_uint_eq(ts->sock.tcp.snd_una, ts->sock.tcp.seq);

/* The wrap-aware ordering invariant: snd_una <= seq */
ck_assert_int_eq(tcp_seq_leq(ts->sock.tcp.snd_una, ts->sock.tcp.seq), 1);
}
END_TEST

START_TEST(test_regression_duplicate_syn_rejected_on_established)
{
struct wolfIP s;
int sd, sd2;
struct wolfIP_sockaddr_in sin;
struct tsocket *listener, *established;
ip4 local_ip = 0x0A000001U;
ip4 remote_ip = 0x0A0000A1U; /* hardcoded in inject_tcp_syn */
uint16_t local_port = 8080;
uint16_t remote_port = 40000; /* hardcoded in inject_tcp_syn */

wolfIP_init(&s);
mock_link_init(&s);
wolfIP_ipconfig_set(&s, local_ip, 0xFFFFFF00U, 0);

/* Create a listening socket that must stay in LISTEN throughout */
sd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_STREAM, WI_IPPROTO_TCP);
ck_assert_int_gt(sd, 0);
memset(&sin, 0, sizeof(sin));
sin.sin_family = AF_INET;
sin.sin_port = ee16(local_port);
sin.sin_addr.s_addr = ee32(local_ip);
wolfIP_sock_bind(&s, sd, (struct wolfIP_sockaddr *)&sin, sizeof(sin));
wolfIP_sock_listen(&s, sd, 4);
listener = &s.tcpsockets[SOCKET_UNMARK(sd)];
ck_assert_int_eq(listener->sock.tcp.state, TCP_LISTEN);

/* Allocate a second socket slot and wire it as ESTABLISHED with the
* same 4-tuple that inject_tcp_syn will use. This simulates a live
* connection that was established by a previous handshake. */
sd2 = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_STREAM, WI_IPPROTO_TCP);
ck_assert_int_gt(sd2, 0);
established = &s.tcpsockets[SOCKET_UNMARK(sd2)];
established->sock.tcp.state = TCP_ESTABLISHED;
established->local_ip = local_ip;
established->remote_ip = remote_ip;
established->if_idx = TEST_PRIMARY_IF;
established->src_port = local_port;
established->dst_port = remote_port;
established->sock.tcp.seq = 1000;
established->sock.tcp.snd_una = 1000;
established->sock.tcp.ack = 2;
established->sock.tcp.cwnd = TCP_MSS;
established->sock.tcp.peer_rwnd = TCP_MSS;
queue_init(&established->sock.tcp.rxbuf, established->rxmem, RXBUF_SIZE, 0);
fifo_init(&established->sock.tcp.txbuf, established->txmem, TXBUF_SIZE);

/* Send a SYN whose tuple matches the established connection above.
* Without the fix (da5c792): listener → TCP_SYN_RCVD.
* With the fix: listener stays in TCP_LISTEN. */
inject_tcp_syn(&s, TEST_PRIMARY_IF, local_ip, local_port);

ck_assert_int_eq(listener->sock.tcp.state, TCP_LISTEN);
}
END_TEST

/* ----------------------------------------------------------------------- */

Suite *wolf_suite(void)
{
Expand Down Expand Up @@ -14749,7 +14842,13 @@ Suite *wolf_suite(void)
suite_add_tcase(s, tc_proto);
tcase_add_test(tc_proto, test_tcp_handshake_and_fin_close_wait);
suite_add_tcase(s, tc_proto);


/* Regression tests for fixes-4 branch */
tcase_add_test(tc_proto, test_regression_snd_una_initialized_on_syn_rcvd);
suite_add_tcase(s, tc_proto);
tcase_add_test(tc_proto, test_regression_duplicate_syn_rejected_on_established);
suite_add_tcase(s, tc_proto);
Comment on lines 14842 to +14850
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suite_add_tcase(s, tc_proto) is called multiple times for the same tc_proto object in this block (including the newly added calls). In Check, adding the same TCase* to a suite more than once can lead to duplicated execution or undefined behavior depending on the library/version. The safer pattern is to add tc_proto to the suite once, and only call tcase_add_test() for each additional test.

Copilot uses AI. Check for mistakes.

tcase_add_test(tc_utils, test_transport_checksum);
suite_add_tcase(s, tc_proto);
tcase_add_test(tc_utils, test_iphdr_set_checksum);
Expand Down
51 changes: 42 additions & 9 deletions src/wolfip.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,10 +119,10 @@ struct wolfIP_icmp_packet;
/* Macros */
#define IS_IP_BCAST(ip) (ip == 0xFFFFFFFF)

#define PKT_FLAG_SENT 0x01
#define PKT_FLAG_ACKED 0x02
#define PKT_FLAG_FIN 0x04
#define PKT_FLAG_RETRANS 0x08
#define PKT_FLAG_SENT 0x01U
#define PKT_FLAG_ACKED 0x02U
#define PKT_FLAG_FIN 0x04U
#define PKT_FLAG_RETRANS 0x08U
#define TX_WRITABLE_THRESHOLD 1

#define TCP_SACK_MAX_BLOCKS 4
Expand Down Expand Up @@ -504,6 +504,16 @@ static uint32_t queue_len(struct queue *q)
return (q->size - 1) - queue_space(q);
}


/* Subtract two TCP sequence numbers: a - b (wraps at 2^32) */
static inline uint32_t tcp_seq_diff(uint32_t a, uint32_t b)
{
if (a >= b)
return a - b;
return UINT32_MAX - (b - a - 1);
Comment on lines +508 to +513
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tcp_seq_diff() returns uint32_t, but at least one call site treats the result as signed (rel < 0). This makes the behavior dependent on an implicit unsigned→signed conversion (implementation-defined when the value is not representable), which can break the “old data behind seq_base is rejected” logic—especially around wrap or large gaps. Consider changing tcp_seq_diff to return an int32_t “relative distance” (the same semantic as the prior (int32_t)(seq - base) approach) and update call sites accordingly, or keep it unsigned but remove signed comparisons and use wrap-aware ordering helpers consistently.

Suggested change
/* Subtract two TCP sequence numbers: a - b (wraps at 2^32) */
static inline uint32_t tcp_seq_diff(uint32_t a, uint32_t b)
{
if (a >= b)
return a - b;
return UINT32_MAX - (b - a - 1);
/* Subtract two TCP sequence numbers: a - b (wraps at 2^32)
* and interpret the result as a signed relative distance. */
static inline int32_t tcp_seq_diff(uint32_t a, uint32_t b)
{
/* Sequence arithmetic is modulo 2^32; cast to int32_t to get
* a signed relative distance in the range [-2^31, 2^31-1]. */
return (int32_t)(a - b);

Copilot uses AI. Check for mistakes.
}


/* Insert data into the queue */
static int queue_insert(struct queue *q, void *data, uint32_t seq, uint32_t len)
{
Expand All @@ -526,7 +536,7 @@ static int queue_insert(struct queue *q, void *data, uint32_t seq, uint32_t len)
/* Sequence arithmetic is modulo 2^32. Use signed relative distance
* so contiguous inserts across wrap are accepted and old data behind
* seq_base is rejected. */
rel = (int32_t)(seq - q->seq_base);
rel = tcp_seq_diff(seq, q->seq_base);
if (rel < 0) {
Comment on lines +539 to 540
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tcp_seq_diff() returns uint32_t, but at least one call site treats the result as signed (rel < 0). This makes the behavior dependent on an implicit unsigned→signed conversion (implementation-defined when the value is not representable), which can break the “old data behind seq_base is rejected” logic—especially around wrap or large gaps. Consider changing tcp_seq_diff to return an int32_t “relative distance” (the same semantic as the prior (int32_t)(seq - base) approach) and update call sites accordingly, or keep it unsigned but remove signed comparisons and use wrap-aware ordering helpers consistently.

Copilot uses AI. Check for mistakes.
/* Old data that is behind the current receive base. */
return -1;
Expand Down Expand Up @@ -1707,7 +1717,7 @@ static void tcp_parse_options(const struct wolfIP_tcp_seg *tcp, uint32_t frame_l
{
const uint8_t *opt = tcp->data;
int claimed_opt_len = (tcp->hlen >> 2) - TCP_HEADER_LEN;
int available_bytes = (int)(frame_len - sizeof(struct wolfIP_tcp_seg));
int available_bytes = (int)frame_len - (int)sizeof(struct wolfIP_tcp_seg);
int opt_len;
const uint8_t *opt_end;

Expand Down Expand Up @@ -1922,7 +1932,7 @@ static void tcp_consume_ooo(struct tsocket *t)
break;
} else {
/* Keep only the still-unacknowledged suffix. */
uint32_t trim = t->sock.tcp.ack - t->sock.tcp.ooo[i].seq;
uint32_t trim = tcp_seq_diff(t->sock.tcp.ack, t->sock.tcp.ooo[i].seq);
memmove(t->sock.tcp.ooo[i].data,
t->sock.tcp.ooo[i].data + trim,
t->sock.tcp.ooo[i].len - trim);
Expand Down Expand Up @@ -2117,14 +2127,14 @@ static void tcp_recv(struct tsocket *t, struct wolfIP_tcp_seg *seg)
if (seg_len == 0)
return;
if (tcp_seq_lt(seq, t->sock.tcp.ack)) {
uint32_t consumed = t->sock.tcp.ack - seq;
uint32_t consumed = tcp_seq_diff(t->sock.tcp.ack, seq);
/* Retransmitted/overlapping data below ACK is already delivered.
* Trim it so only bytes above ACK participate in hole handling. */
if (consumed >= seg_len) {
tcp_send_ack(t);
return;
}
seq += consumed;
seq = tcp_seq_inc(seq, consumed);
payload += consumed;
seg_len -= consumed;
}
Expand Down Expand Up @@ -2821,12 +2831,34 @@ static void tcp_input(struct wolfIP *S, unsigned int if_idx,
ip4 syn_dst = ee32(tcp->ip.dst);
int dst_match = 0;
unsigned int dst_if;
int dup_found = 0;

if (syn_dst == IPADDR_ANY)
continue;
if (t->bound_local_ip != IPADDR_ANY && t->bound_local_ip != syn_dst)
continue;

/* Reject SYNs that match an already-active connection
* with the same tuple (local_port, remote_port, remote_ip), that is already
* in the established state.
* */
for (int k = 0; k < MAX_TCPSOCKETS; k++) {
struct tsocket *tk = &S->tcpsockets[k];
if (tk == t)
continue;
if (tk->sock.tcp.state <= TCP_LISTEN ||
tk->sock.tcp.state == TCP_CLOSED)
continue;
if (tk->src_port == t->src_port &&
tk->dst_port == ee16(tcp->src_port) &&
tk->remote_ip == ee32(tcp->ip.src)) {
dup_found = 1;
break;
}
}
Comment on lines +2841 to +2858
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The duplicate-SYN filter does not include the local IP in the tuple comparison. If the stack can listen on multiple local IPs (or IPADDR_ANY) with the same local port, a SYN to a different local IP could be incorrectly rejected just because an established connection exists with the same remote endpoint and ports. Include the appropriate local identifier(s) in the match (e.g., tk->local_ip/syn_dst, and potentially if_idx if that’s part of the effective demux key in this stack) so only truly identical connections are considered duplicates.

Copilot uses AI. Check for mistakes.
if (dup_found)
continue;

dst_if = wolfIP_if_for_local_ip(S, syn_dst, &dst_match);
if (!dst_match)
continue;
Expand All @@ -2836,6 +2868,7 @@ static void tcp_input(struct wolfIP *S, unsigned int if_idx,
t->sock.tcp.state = TCP_SYN_RCVD;
t->sock.tcp.ack = tcp_seq_inc(ee32(tcp->seq), 1);
t->sock.tcp.seq = wolfIP_getrandom();
t->sock.tcp.snd_una = t->sock.tcp.seq;
t->dst_port = ee16(tcp->src_port);
t->remote_ip = ee32(tcp->ip.src);
t->events |= CB_EVENT_READABLE; /* Keep flag until application calls accept */
Expand Down