Re: [RFC][PATCH v2 19/31] timers: net: Use del_timer_shutdown() before freeing timer

From: Paolo Abeni
Date: Sun Oct 30 2022 - 13:23:13 EST


On Fri, 2022-10-28 at 15:46 -0700, Jakub Kicinski wrote:
> On Fri, 28 Oct 2022 18:31:49 -0400 Steven Rostedt wrote:
> > Could someone from networking confirm (or deny) that the timer being
> > removed in sk_stop_timer() will no longer be used even if del_timer()
> > returns false?
> >
> > net/core/sock.c:
> >
> > void sk_stop_timer(struct sock *sk, struct timer_list* timer)
> > {
> > if (del_timer(timer))
> > __sock_put(sk);
> > }
> >
> > If this is the case, then I'll add the following interface:
> >
> > del_timer_sync_shutdown() // the common case which syncs
> >
> > del_timer_shutdown() // the uncommon case, that returns immediately
> > // used for those cases that add extra code to
> > // handle it, like sk_stop_timer()
>
> Sorry too many bugs at once :)
>
> FWIW Paolo was saying privately earlier today that he spotted some cases
> of reuse, he gave an example of ccid2_hc_tx_packet_recv()

For the records, there are other cases, e.g. after sk_stop_timer() in
clear_3rdack_retransmission() (mptcp code) the timer can be-rearmed
without re-initializing. I *think* there are more of such use in the
in ax25/rose code.

> So we can't convert all cases of sk_stop_timer() in one fell swoop :(

On the positive side, I think converting the sk_stop_timer in
inet_csk_clear_xmit_timers() should be safe and should cover the issue
reported by Guenter

Cheers,

Paolo