Skip to content

Commit 9c0a465

Browse files
nhormangregkh
authored andcommitted
sctp: Don't add the shutdown timer if its already been added
[ Upstream commit 20a785a ] 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 [f418df70] 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 <nhorman@tuxdriver.com> CC: Vlad Yasevich <vyasevich@gmail.com> CC: "David S. Miller" <davem@davemloft.net> CC: jere.leppanen@nokia.com CC: marcelo.leitner@gmail.com CC: netdev@vger.kernel.org Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent ea17010 commit 9c0a465

1 file changed

Lines changed: 11 additions & 3 deletions

File tree

net/sctp/sm_sideeffect.c

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1537,9 +1537,17 @@ static int sctp_cmd_interpreter(enum sctp_event event_type,
15371537
timeout = asoc->timeouts[cmd->obj.to];
15381538
BUG_ON(!timeout);
15391539

1540-
timer->expires = jiffies + timeout;
1541-
sctp_association_hold(asoc);
1542-
add_timer(timer);
1540+
/*
1541+
* SCTP has a hard time with timer starts. Because we process
1542+
* timer starts as side effects, it can be hard to tell if we
1543+
* have already started a timer or not, which leads to BUG
1544+
* halts when we call add_timer. So here, instead of just starting
1545+
* a timer, if the timer is already started, and just mod
1546+
* the timer with the shorter of the two expiration times
1547+
*/
1548+
if (!timer_pending(timer))
1549+
sctp_association_hold(asoc);
1550+
timer_reduce(timer, jiffies + timeout);
15431551
break;
15441552

15451553
case SCTP_CMD_TIMER_RESTART:

0 commit comments

Comments
 (0)