Skip to content

Fix Q-Block re-entrancy in rate-limiting pacing #1973

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
yavuzhaliloglu wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

Fix Q-Block re-entrancy in rate-limiting pacing #1973

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
269f33b
Fix Q-Block re-entrancy in rate-limiting pacing
yavuzhaliloglu Apr 30, 2026
b63a789
Potential fix for pull request finding
yavuzhaliloglu Apr 30, 2026
80416e1
Save and restore is_rate_limiting in pacing loop to prevent re-entran…
yavuzhaliloglu Apr 30, 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
7 changes: 7 additions & 0 deletions include/coap3/coap_session_internal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,13 @@ struct coap_session_t {
uint64_t rl_ticks_per_packet; /**< If not 0, rate limit NON to ticks per packet */
coap_tick_t last_tx; /**< Last time a ratelimited packet is sent */
uint8_t is_rate_limiting; /**< Currently NON rate limiting */
uint8_t is_doing_q_block; /**< Currently processing a Q-Block response
handler (e.g. 4.08 retransmit, 2.31
Continue triggering next MAX_PAYLOADS
set). Forces coap_send_internal() to
still apply pacing even though
is_rate_limiting is set by an outer
wait loop in a recursive io_process. */
Comment thread
yavuzhaliloglu marked this conversation as resolved.
Outdated
uint32_t ping_failed; /**< Ping failure count */
};

Expand Down
62 changes: 54 additions & 8 deletions src/coap_block.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2245,6 +2245,16 @@ coap_block_check_q_block1_xmit(coap_session_t *session, coap_tick_t now, coap_ti
int ret = 0;

*tim_rem = COAP_MAX_DELAY_TICKS;
if (session->is_doing_q_block) {
/*
* A Q-Block send is already in flight on this session (e.g. an outer
* coap_send_q_blocks() iterating with rate-limit pacing, or a 4.08 /
* 2.31 response handler). Skip the timer-driven next-MAX_PAYLOAD-set
* push so we do not start an overlapping coap_send_q_blocks() for the
* same lg_xmit, which would re-send already-queued blocks.
*/
return ret;
}
LL_FOREACH_SAFE(session->lg_xmit, lg_xmit, q) {
coap_tick_t non_timeout = lg_xmit->non_timeout_random_ticks;

Expand All @@ -2265,7 +2275,9 @@ coap_block_check_q_block1_xmit(coap_session_t *session, coap_tick_t now, coap_ti
block.num = (uint32_t)(lg_xmit->offset / chunk);
block.m = lg_xmit->offset + chunk < lg_xmit->data_info->length;
block.szx = lg_xmit->blk_size;
session->is_doing_q_block = 1;
coap_send_q_blocks(session, lg_xmit, block, lg_xmit->sent_pdu, COAP_SEND_SKIP_PDU);
session->is_doing_q_block = 0;
if (*tim_rem > non_timeout) {
*tim_rem = non_timeout;
ret = 1;
Expand Down Expand Up @@ -2309,6 +2321,11 @@ coap_block_check_q_block2_xmit(coap_session_t *session, coap_tick_t now, coap_ti
int ret = 0;

*tim_rem = (coap_tick_t)-1;
if (session->is_doing_q_block) {
/* See coap_block_check_q_block1_xmit() — skip while a Q-Block send is
already in flight on this session to avoid overlapping sends. */
return ret;
}
LL_FOREACH_SAFE(session->lg_xmit, lg_xmit, q) {
coap_tick_t non_timeout = lg_xmit->non_timeout_random_ticks;

Expand All @@ -2329,8 +2346,11 @@ coap_block_check_q_block2_xmit(coap_session_t *session, coap_tick_t now, coap_ti
block.num = (uint32_t)(lg_xmit->offset / chunk);
block.m = lg_xmit->offset + chunk < lg_xmit->data_info->length;
block.szx = lg_xmit->blk_size;
if (block.num == (uint32_t)lg_xmit->last_block)
if (block.num == (uint32_t)lg_xmit->last_block) {
session->is_doing_q_block = 1;
coap_send_q_blocks(session, lg_xmit, block, lg_xmit->sent_pdu, COAP_SEND_SKIP_PDU);
session->is_doing_q_block = 0;
}
if (*tim_rem > non_timeout) {
*tim_rem = non_timeout;
ret = 1;
Expand Down Expand Up @@ -3908,8 +3928,14 @@ coap_handle_response_send_block(coap_session_t *session, coap_pdu_t *sent,
#if COAP_Q_BLOCK_SUPPORT
if (lg_xmit->option == COAP_OPTION_Q_BLOCK1 &&
pdu->type == COAP_MESSAGE_NON) {
if (coap_send_q_block1(session, block, pdu,
COAP_SEND_INC_PDU) == COAP_INVALID_MID)
coap_mid_t q_mid;
/* Save/restore — see 4.08 handler below for rationale. */
uint8_t saved_q_block = session->is_doing_q_block;

session->is_doing_q_block = 1;
q_mid = coap_send_q_block1(session, block, pdu, COAP_SEND_INC_PDU);
session->is_doing_q_block = saved_q_block;
if (q_mid == COAP_INVALID_MID)
goto fail_body;
return 1;
} else if (coap_send_internal(session, pdu, NULL) == COAP_INVALID_MID)
Expand Down Expand Up @@ -3990,15 +4016,25 @@ coap_handle_response_send_block(coap_session_t *session, coap_pdu_t *sent,
uint64_t token;
uint8_t ltoken[8];
size_t ltoken_length;
/*
* Save/restore is_doing_q_block: this handler can run nested
* inside an outer coap_send_q_blocks() (timer-driven next
* MAX_PAYLOAD set, etc.) that already set the flag. Plain
* set-then-clear would zero the outer's flag on our exit and
* let coap_block_check_q_block1_xmit() spawn an overlapping
* coap_send_q_blocks() while the outer is still iterating.
*/
uint8_t saved_q_block = session->is_doing_q_block;

session->is_doing_q_block = 1;
for (i = 0; (bp < data + length) &&
i < COAP_MAX_PAYLOADS(session); i++) {
if ((*bp & 0xc0) != 0x00) /* uint(value) */
goto fail_cbor;
goto fail_cbor_q_block;
block.num = derive_cbor_value(&bp, data + length - bp);
coap_log_debug("Q-Block1: Missing block %d\n", block.num);
if (block.num > (1 << 20) -1)
goto fail_cbor;
goto fail_cbor_q_block;
block.m = (block.num + 1) * chunk < lg_xmit->data_info->length;
block.szx = lg_xmit->blk_size;

Expand All @@ -4007,8 +4043,10 @@ coap_handle_response_send_block(coap_session_t *session, coap_pdu_t *sent,
ltoken_length = coap_encode_var_safe8(ltoken, sizeof(token), token);
pdu = coap_pdu_duplicate_lkd(lg_xmit->sent_pdu, session, ltoken_length,
ltoken, NULL, COAP_BOOL_FALSE);
if (!pdu)
if (!pdu) {
session->is_doing_q_block = saved_q_block;
goto fail_body;
}

coap_update_option(pdu, lg_xmit->option,
coap_encode_var_safe(buf, sizeof(buf),
Expand All @@ -4021,12 +4059,20 @@ coap_handle_response_send_block(coap_session_t *session, coap_pdu_t *sent,
lg_xmit->data_info->length,
lg_xmit->data_info->data,
block.num,
block.szx))
block.szx)) {
session->is_doing_q_block = saved_q_block;
goto fail_body;
if (coap_send_internal(session, pdu, NULL) == COAP_INVALID_MID)
}
if (coap_send_internal(session, pdu, NULL) == COAP_INVALID_MID) {
session->is_doing_q_block = saved_q_block;
goto fail_body;
}
}
session->is_doing_q_block = saved_q_block;
return 1;
fail_cbor_q_block:
session->is_doing_q_block = saved_q_block;
goto fail_cbor;
}
fail_cbor:
coap_log_info("Invalid application/missing-blocks+cbor-seq\n");
Expand Down
2 changes: 1 addition & 1 deletion src/coap_net.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2044,7 +2044,7 @@ coap_send_internal(coap_session_t *session, coap_pdu_t *pdu, coap_pdu_t *request
if (pdu->type == COAP_MESSAGE_NON && session->rl_ticks_per_packet) {
coap_tick_t now;

if (!session->is_rate_limiting) {
if (!session->is_rate_limiting || session->is_doing_q_block) {
coap_ticks(&now);
Comment thread
yavuzhaliloglu marked this conversation as resolved.
while (1) {
uint32_t timeout_ms;
Comment thread
yavuzhaliloglu marked this conversation as resolved.
Expand Down
Loading