Skip to content

Commit

Permalink
BUG/MEDIUM: stconn: Only consider I/O timers to update stream's expir…
Browse files Browse the repository at this point in the history
…ation date

In sc_notify(), it remained a case where it was possible to set an
expiration date on the stream in the past, leading to a crash because of a
BUG_ON(). This must never happen of course.

In sc_notify(), The stream's expiration may be updated in case no wakeup
conditions are encoutered. In that case, we must take care to never set an
expiration date in the past. However, it appeared there was still a
condition to do so. This code is based on an implicit postulate: the
stream's expiration date must always be set when we leave
process_stream(). It was true since the 2.9. But in 3.0, the buffer
allocation mechanism was improved and on an alloc failure in
process_stream(), the stream is inserted in a wait-list and its expiration
date is set to TICK_ETERNITY. With the good timing, and an analysis
expiration date set on a channel, it is possible to set the stream's
expiration date in past.

After analysis, it appeared that the proper way to fix the issue is to only
evaluate I/O timers (read and write timeout) and not stream's timers
(analase_exp or conn_exp) because only I/O timers may have changed since the
last process_stream() call.

This patch must be backported as far as 3.0 to fix the issue. But it is
probably a good idea to also backported it as far as 2.8.
  • Loading branch information
capflam committed Dec 16, 2024
1 parent e3b760e commit 4f32d03
Showing 1 changed file with 3 additions and 5 deletions.
8 changes: 3 additions & 5 deletions src/stconn.c
Original file line number Diff line number Diff line change
Expand Up @@ -1199,17 +1199,15 @@ void sc_notify(struct stconn *sc)
task_wakeup(task, TASK_WOKEN_IO);
}
else {
/* Update expiration date for the task and requeue it if not already expired */
/* Update expiration date for the task and requeue it if not already expired.
* Only I/O timeouts are evaluated. The stream is responsible of others.
*/
if (!tick_is_expired(task->expire, now_ms)) {
task->expire = tick_first(task->expire, sc_ep_rcv_ex(sc));
task->expire = tick_first(task->expire, sc_ep_snd_ex(sc));
task->expire = tick_first(task->expire, sc_ep_rcv_ex(sco));
task->expire = tick_first(task->expire, sc_ep_snd_ex(sco));
task->expire = tick_first(task->expire, ic->analyse_exp);
task->expire = tick_first(task->expire, oc->analyse_exp);
task->expire = tick_first(task->expire, __sc_strm(sc)->conn_exp);

/* WARNING: Don't forget to remove this BUG_ON before 2.9.0 */
BUG_ON(tick_is_expired(task->expire, now_ms));
task_queue(task);
}
Expand Down

0 comments on commit 4f32d03

Please sign in to comment.