Re: [RFC] net: Replace del_timer() with del_timer_sync()

From: Deepak
Date: Mon Aug 11 2014 - 06:26:21 EST

Thanks for the clarification Eric.
I re-analysed the code and found that sk_stop_timer() is called under lock_sock(sk)/bh_lock_sock(sk) so we can not replace del_timer() with del_timer_sync() here and will lead to dead-lock as you suggested .

Deepak Das

On Thursday 07 August 2014 10:18 PM, Eric Dumazet wrote:
On Thu, 2014-08-07 at 15:15 +0000, Das, Deepak wrote:

Please do not top post on netdev, thanks.

I apologies for not explaining the scenario previously.

sk_stop_timer() is used to stop the tcp timers with expiry callback
tcp_write_timer(), tcp_delack_timer(), tcp_keepalive_timer(), ...
del_timer() is used to stop the the timer in sk_stop_timer(), which
might return a non-zero result even if one of these timer handler functions
(tcp_write_timer(), tcp_delack_timer(), tcp_keepalive_timer(), ...)
is already executing on another processor.

Following is the possible scenario :-
on CPU #0: sk_stop_timer() decrements the sk->sk_refcnt if del_timer(timer)
returns non-zero.

on CPU #1: If a timer handler callback runs then it also calls sock_put(sk)
which decrements sk->sk_refcnt and if the sk_refcnt becomes zero it frees the
structure sock pointed to by sk.

if the sk->sk_refcnt decrements twice then that will cause a mismatch in the
number of "puts" and "holds" resulting in a malfunction of the sk->sk_refcnt mechanism.
Not at all.

There is no mismatch, sk_refcnt is decremented once in all cases.

I believe you misunderstood del_timer_sync() / del_timer() behaviors and

In the case you describe, del_timer() should return 0, and timer
function will call sock_put() to decrement socket refcount.

The problem' of del_timer() is the following :

When/If it returns 0, another cpu _might_ be running the timer, we have
no guarantee timer function is completed.

For sockets, we do not care, because the active timer owns a refcount on
the socket. When timer is finally completed, refcount will be released.

The solution is to use del_timer_sync() instead of del_timer()
because del_timer_sync() will wait for timer handler functions to
complete execution.
Except that some sk_stop_timer() callers hold the socket lock, so the
timer will deadlock trying to acquire it.

yes, we are facing some memory corruption issues due to access of already released
struct sock in our environment. Our memory corruption issue looks like memory locations
being decremented which could be consistent with a rogue decrement of a reference counter.
Is 'Your environment' some out of tree module or is it part of standard
linux kernel ?

similar suggestion is also made by Dean Jenkins in rfcomm_dlc_clear_timer() and accepted by Marcel.
Fix might be good in this case, but the changelog is completely bogus.

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at