From: Florian Westphal <fw@strlen.de>
To: Hillf Danton <hdanton@sina.com>
Cc: Florian Westphal <fw@strlen.de>,
netfilter-devel@vger.kernel.org, linux-kernel@vger.kernel.org,
netdev@vger.kernel.org, syzkaller-bugs@googlegroups.com,
syzbot+4fd66a69358fc15ae2ad@syzkaller.appspotmail.com
Subject: Re: [PATCH nf] netfilter: nf_tables: unconditionally flush pending work before notifier
Date: Wed, 3 Jul 2024 15:01:07 +0200 [thread overview]
Message-ID: <20240703130107.GB29258@breakpoint.cc> (raw)
In-Reply-To: <20240703120913.2981-1-hdanton@sina.com>
Hillf Danton <hdanton@sina.com> wrote:
> On Wed, 3 Jul 2024 12:52:15 +0200 Florian Westphal <fw@strlen.de>
> > Hillf Danton <hdanton@sina.com> wrote:
> > > Given trans->table goes thru the lifespan of trans, your proposal is a bandaid
> > > if trans outlives table.
> >
> > trans must never outlive table.
> >
> What is preventing trans from being freed after closing sock, given
> trans is freed in workqueue?
>
> close sock
> queue work
The notifier acquires the transaction mutex, locking out all other
transactions, so no further transactions requests referencing
the table can be queued.
The work queue is flushed before potentially ripping the table
out. After this, no transactions referencing the table can exist
anymore; the only transactions than can still be queued are those
coming from a different netns, and tables are scoped per netns.
Table is torn down. Transaction mutex is released.
Next transaction from userspace can't find the table anymore (its gone),
so no more transactions can be queued for this table.
As I wrote in the commit message, the flush is dumb, this should first
walk to see if there is a matching table to be torn down, and then flush
work queue once before tearing the table down.
But its better to clearly split bug fix and such a change.
next prev parent reply other threads:[~2024-07-03 13:01 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-07-01 20:19 [syzbot] [netfilter?] KASAN: slab-use-after-free Read in nf_tables_trans_destroy_work syzbot
2024-07-02 14:08 ` [PATCH nf] netfilter: nf_tables: unconditionally flush pending work before notifier Florian Westphal
2024-07-03 10:35 ` Hillf Danton
2024-07-03 10:52 ` Florian Westphal
2024-07-03 12:09 ` Hillf Danton
2024-07-03 13:01 ` Florian Westphal [this message]
2024-07-04 10:35 ` Hillf Danton
2024-07-04 10:54 ` Florian Westphal
2024-07-05 10:48 ` Hillf Danton
2024-07-05 11:02 ` Florian Westphal
2024-07-07 7:56 ` Hillf Danton
2024-07-07 8:08 ` Florian Westphal
2024-07-08 10:56 ` Hillf Danton
2024-07-08 11:58 ` Florian Westphal
2024-07-08 12:17 ` Hillf Danton
2024-07-08 12:43 ` Florian Westphal
2024-07-05 11:18 ` [syzbot] [netfilter?] KASAN: slab-use-after-free Read in nf_tables_trans_destroy_work syzbot
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=20240703130107.GB29258@breakpoint.cc \
--to=fw@strlen.de \
--cc=hdanton@sina.com \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=netfilter-devel@vger.kernel.org \
--cc=syzbot+4fd66a69358fc15ae2ad@syzkaller.appspotmail.com \
--cc=syzkaller-bugs@googlegroups.com \
/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).