netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Paolo Abeni <pabeni@redhat.com>
To: Jakub Kicinski <kuba@kernel.org>, Steven Rostedt <rostedt@goodmis.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	linux-kernel@vger.kernel.org,
	Thomas Gleixner <tglx@linutronix.de>,
	Stephen Boyd <sboyd@kernel.org>,
	Guenter Roeck <linux@roeck-us.net>,
	Jesse Brandeburg <jesse.brandeburg@intel.com>,
	Tony Nguyen <anthony.l.nguyen@intel.com>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Mirko Lindner <mlindner@marvell.com>,
	Stephen Hemminger <stephen@networkplumber.org>,
	Martin KaFai Lau <martin.lau@kernel.org>,
	Alexei Starovoitov <ast@kernel.org>,
	Kuniyuki Iwashima <kuniyu@amazon.com>,
	Pavel Begunkov <asml.silence@gmail.com>,
	Menglong Dong <imagedong@tencent.com>,
	linux-usb@vger.kernel.org, linux-wireless@vger.kernel.org,
	bridge@lists.linux-foundation.org,
	netfilter-devel@vger.kernel.org, coreteam@netfilter.org,
	lvs-devel@vger.kernel.org, linux-afs@lists.infradead.org,
	linux-nfs@vger.kernel.org, tipc-discussion@lists.sourceforge.net
Subject: Re: [RFC][PATCH v2 19/31] timers: net: Use del_timer_shutdown() before freeing timer
Date: Sun, 30 Oct 2022 18:22:03 +0100	[thread overview]
Message-ID: <27a6a587fee5e9172e41acd16ae1bc1f556fdbd7.camel@redhat.com> (raw)
In-Reply-To: <20221028154617.3c63ba68@kernel.org>

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


  reply	other threads:[~2022-10-30 17:23 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20221027150525.753064657@goodmis.org>
2022-10-27 15:05 ` [RFC][PATCH v2 19/31] timers: net: Use del_timer_shutdown() before freeing timer Steven Rostedt
2022-10-27 19:55   ` Steven Rostedt
2022-10-27 20:15     ` Linus Torvalds
2022-10-27 20:34       ` Steven Rostedt
2022-10-27 20:48         ` Linus Torvalds
2022-10-27 21:07           ` Steven Rostedt
2022-10-27 21:15             ` Steven Rostedt
2022-10-27 22:35             ` Steven Rostedt
2022-10-28 22:31               ` Steven Rostedt
2022-10-28 22:46                 ` Jakub Kicinski
2022-10-30 17:22                   ` Paolo Abeni [this message]
2022-11-03 21:51                     ` Steven Rostedt
2022-11-04  0:00                       ` Eric Dumazet
2022-11-04  5:51                         ` Steven Rostedt
2022-11-04 16:14                           ` Guenter Roeck
2022-10-27 21:07         ` Steven Rostedt
2022-10-28 15:16           ` Guenter Roeck
2022-10-27 23:13 Guenter Roeck

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=27a6a587fee5e9172e41acd16ae1bc1f556fdbd7.camel@redhat.com \
    --to=pabeni@redhat.com \
    --cc=anthony.l.nguyen@intel.com \
    --cc=asml.silence@gmail.com \
    --cc=ast@kernel.org \
    --cc=bridge@lists.linux-foundation.org \
    --cc=coreteam@netfilter.org \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=imagedong@tencent.com \
    --cc=jesse.brandeburg@intel.com \
    --cc=kuba@kernel.org \
    --cc=kuniyu@amazon.com \
    --cc=linux-afs@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=lvs-devel@vger.kernel.org \
    --cc=martin.lau@kernel.org \
    --cc=mlindner@marvell.com \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=rostedt@goodmis.org \
    --cc=sboyd@kernel.org \
    --cc=stephen@networkplumber.org \
    --cc=tglx@linutronix.de \
    --cc=tipc-discussion@lists.sourceforge.net \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).