netdev - Re: [PATCH nf-next] netns: add and use net_ns

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170613180713.GA10130@breakpoint.cc>
Date:   Tue, 13 Jun 2017 20:07:13 +0200
From:   Florian Westphal <fw@...len.de>
To:     Cong Wang <xiyou.wangcong@...il.com>
Cc:     Florian Westphal <fw@...len.de>,
        "Eric W. Biederman" <ebiederm@...ssion.com>,
        netfilter-devel@...r.kernel.org,
        Linux Kernel Network Developers <netdev@...r.kernel.org>
Subject: Re: [PATCH nf-next] netns: add and use net_ns_barrier

Cong Wang <xiyou.wangcong@...il.com> wrote:
> On Mon, Jun 12, 2017 at 11:16 PM, Florian Westphal <fw@...len.de> wrote:
> > Cong Wang <xiyou.wangcong@...il.com> wrote:
> >> On Thu, Jun 1, 2017 at 1:52 AM, Florian Westphal <fw@...len.de> wrote:
> >> > Joe described it nicely, problem is that after unload we may have
> >> > conntracks that still have a nf_conn_help extension attached that
> >> > has a pointer to a structure that resided in the (unloaded) module.
> >>
> >> Why not hold a refcnt for its module?
> >
> > That would work as well.
> >
> > I'm not sure its nice to disallow rmmod of helper modules if they are
> > used by a connection however.
> 
> I am _not_ suggesting to disallow rmmod.

My point was that if you hold reference counts to the module users
will need to manually flush conntrack table (or at least manually
remove affected connections), else rmmod won't work as refcount might be
gt 0.

> > Right now you can "rmmod nf_conntrack_foo" at any time and this should
> > work just fine without first having to flush affected conntracks
> > manually.
> 
> My point is that since netns wq could invoke code of that module,
> why it doesn't hold a refcnt of that module?

*shrug*, I did not write this stuff.

Historically it wasn't needed because we just clear out the helper area
in the affected conntracks (i.e, future packets are not inspected by
the helper anymore).

When conntracks were made per-netns this problem was added as we're not
guaranteed to see all net namespace because module_exit and netns cleanup
can run concurrently.

We can still use the "old" model if we guarantee that we wait for
netns cleanup to finish (which is what this patch does).

The alternative, as you pointed out, is to take a module reference for
each conntrack that uses the helper (and put again when connection is
destroyed).

I don't really care that much except that if we go for the latter
solution users cannot "just rmmod" the module anymore but might have
to manually remove the affected connections first.

Pablo, I can rework things to do module get/put but I ask for explicit
instructions to do so (I prefer the "barrier" approach).