Hi Pablo,
On Sat, Mar 18, 2017 at 11:46 AM, <fgao@xxxxxxxxxx> wrote:
> From: Gao Feng <fgao@xxxxxxxxxx>
>
> There is no rcu_read_lock during ctlink gets the helper and inserts the
> expectation. So there is one possible use-after-free issue when unload
> the helper module.
>
> For example:
>
> CPU1 CPU2
> ctlink gets the helper
> helper module unload and remove all expectations
> insert the expectation
>
> Now there is one expectation which references one helper whose module is
> unloaded.
>
> Signed-off-by: Gao Feng <fgao@xxxxxxxxxx>
> ---
> net/netfilter/nf_conntrack_netlink.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
> index 6806b5e..f6d1d63 100644
> --- a/net/netfilter/nf_conntrack_netlink.c
> +++ b/net/netfilter/nf_conntrack_netlink.c
> @@ -3133,23 +3133,27 @@ static int ctnetlink_del_expect(struct net *net, struct sock *ctnl,
> return -ENOENT;
> ct = nf_ct_tuplehash_to_ctrack(h);
>
> + rcu_read_lock();
> if (cda[CTA_EXPECT_HELP_NAME]) {
> const char *helpname = nla_data(cda[CTA_EXPECT_HELP_NAME]);
>
> helper = __nf_conntrack_helper_find(helpname, u3,
> nf_ct_protonum(ct));
> if (helper == NULL) {
> + rcu_read_unlock();
> #ifdef CONFIG_MODULES
> if (request_module("nfct-helper-%s", helpname) < 0) {
> err = -EOPNOTSUPP;
> goto err_ct;
> }
> + rcu_read_lock();
> helper = __nf_conntrack_helper_find(helpname, u3,
> nf_ct_protonum(ct));
> if (helper) {
> err = -EAGAIN;
> - goto err_ct;
> + goto err_rcu;
> }
> + rcu_read_unlock();
> #endif
> err = -EOPNOTSUPP;
> goto err_ct;
> @@ -3159,11 +3163,13 @@ static int ctnetlink_del_expect(struct net *net, struct sock *ctnl,
> exp = ctnetlink_alloc_expect(cda, ct, helper, &tuple, &mask);
> if (IS_ERR(exp)) {
> err = PTR_ERR(exp);
> - goto err_ct;
> + goto err_rcu;
> }
>
> err = nf_ct_expect_related_report(exp, portid, report);
> nf_ct_expect_put(exp);
> +err_rcu:
> + rcu_read_unlock();
> err_ct:
> nf_ct_put(ct);
> return err;
> --
> 1.9.1
>
>
This patch covers part fix of the another patch “Fix possible panic
caused by invoking expectfn unloaded” whose link is
https://patchwork.ozlabs.org/patch/738566/ about RCU lock.
Because the current patch also fixes the use-after-free helper issue,
so I think it is better to commit the two patches.
The first is current one, it fixes the use-after-free bug in
ctnetlink_create_expect.
The second is used to fixes the expectfn issue that we need to remove
all expectations who reference the expectfn when unload the module.
I am thinking about that if there is one better solution than my
original patch.
Regards
Feng
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
