Skip to content

Commit 20a785a

Browse files
nhormandavem330
authored andcommitted
sctp: Don't add the shutdown timer if its already been added
This BUG halt was reported a while back, but the patch somehow got missed: PID: 2879 TASK: c16adaa0 CPU: 1 COMMAND: "sctpn" #0 [f418dd28] crash_kexec at c04a7d8c #1 [f418dd7c] oops_end at c0863e02 #2 [f418dd90] do_invalid_op at c040aaca #3 [f418de28] error_code (via invalid_op) at c08631a5 EAX: f34baac0 EBX: 00000090 ECX: f418deb0 EDX: f5542950 EBP: 00000000 DS: 007b ESI: f34ba800 ES: 007b EDI: f418dea0 GS: 00e0 CS: 0060 EIP: c046fa5e ERR: ffffffff EFLAGS: 00010286 #4 [f418de5c] add_timer at c046fa5e #5 [f418de68] sctp_do_sm at f8db8c77 [sctp] #6 [f418df30] sctp_primitive_SHUTDOWN at f8dcc1b5 [sctp] #7 [f418df48] inet_shutdown at c080baf9 #8 [f418df5c] sys_shutdown at c079eedf #9 [f418df7] sys_socketcall at c079fe88 EAX: ffffffda EBX: 0000000d ECX: bfceea90 EDX: 0937af98 DS: 007b ESI: 0000000c ES: 007b EDI: b7150ae4 SS: 007b ESP: bfceea7c EBP: bfceeaa8 GS: 0033 CS: 0073 EIP: b775c424 ERR: 00000066 EFLAGS: 00000282 It appears that the side effect that starts the shutdown timer was processed multiple times, which can happen as multiple paths can trigger it. This of course leads to the BUG halt in add_timer getting called. Fix seems pretty straightforward, just check before the timer is added if its already been started. If it has mod the timer instead to min(current expiration, new expiration) Its been tested but not confirmed to fix the problem, as the issue has only occured in production environments where test kernels are enjoined from being installed. It appears to be a sane fix to me though. Also, recentely, Jere found a reproducer posted on list to confirm that this resolves the issues Signed-off-by: Neil Horman <[email protected]> CC: Vlad Yasevich <[email protected]> CC: "David S. Miller" <[email protected]> CC: [email protected] CC: [email protected] CC: [email protected] Acked-by: Marcelo Ricardo Leitner <[email protected]> Signed-off-by: David S. Miller <[email protected]>
1 parent c0bbbdc commit 20a785a

File tree

1 file changed

+11
-3
lines changed

1 file changed

+11
-3
lines changed

net/sctp/sm_sideeffect.c

+11-3
Original file line numberDiff line numberDiff line change
@@ -1523,9 +1523,17 @@ static int sctp_cmd_interpreter(enum sctp_event_type event_type,
15231523
timeout = asoc->timeouts[cmd->obj.to];
15241524
BUG_ON(!timeout);
15251525

1526-
timer->expires = jiffies + timeout;
1527-
sctp_association_hold(asoc);
1528-
add_timer(timer);
1526+
/*
1527+
* SCTP has a hard time with timer starts. Because we process
1528+
* timer starts as side effects, it can be hard to tell if we
1529+
* have already started a timer or not, which leads to BUG
1530+
* halts when we call add_timer. So here, instead of just starting
1531+
* a timer, if the timer is already started, and just mod
1532+
* the timer with the shorter of the two expiration times
1533+
*/
1534+
if (!timer_pending(timer))
1535+
sctp_association_hold(asoc);
1536+
timer_reduce(timer, jiffies + timeout);
15291537
break;
15301538

15311539
case SCTP_CMD_TIMER_RESTART:

0 commit comments

Comments
 (0)