Skip to content

Commit

Permalink
netfilter: nfnl_cthelper: reject del request if helper obj is in use
Browse files Browse the repository at this point in the history
We can still delete the ct helper even if it is in use, this will cause
a use-after-free error. In more detail, I mean:
  # nfct helper add ssdp inet udp
  # iptables -t raw -A OUTPUT -p udp -j CT --helper ssdp
  # nfct helper delete ssdp //--> oops, succeed!
  BUG: unable to handle kernel paging request at 000026ca
  IP: 0x26ca
  [...]
  Call Trace:
   ? ipv4_helper+0x62/0x80 [nf_conntrack_ipv4]
   nf_hook_slow+0x21/0xb0
   ip_output+0xe9/0x100
   ? ip_fragment.constprop.54+0xc0/0xc0
   ip_local_out+0x33/0x40
   ip_send_skb+0x16/0x80
   udp_send_skb+0x84/0x240
   udp_sendmsg+0x35d/0xa50

So add reference count to fix this issue, if ct helper is used by
others, reject the delete request.

Apply this patch:
  # nfct helper delete ssdp
  nfct v1.4.3: netlink error: Device or resource busy

Signed-off-by: Liping Zhang <zlpnobody@gmail.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
  • Loading branch information
Liping Zhang authored and Pablo Neira Ayuso committed May 15, 2017
1 parent d91fc59 commit 9338d7b
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 6 deletions.
2 changes: 2 additions & 0 deletions include/net/netfilter/nf_conntrack_helper.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

#ifndef _NF_CONNTRACK_HELPER_H
#define _NF_CONNTRACK_HELPER_H
#include <linux/refcount.h>
#include <net/netfilter/nf_conntrack.h>
#include <net/netfilter/nf_conntrack_extend.h>
#include <net/netfilter/nf_conntrack_expect.h>
Expand All @@ -26,6 +27,7 @@ struct nf_conntrack_helper {
struct hlist_node hnode; /* Internal use. */

char name[NF_CT_HELPER_NAME_LEN]; /* name of the module */
refcount_t refcnt;
struct module *me; /* pointer to self */
const struct nf_conntrack_expect_policy *expect_policy;

Expand Down
6 changes: 6 additions & 0 deletions net/netfilter/nf_conntrack_helper.c
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,10 @@ nf_conntrack_helper_try_module_get(const char *name, u16 l3num, u8 protonum)
#endif
if (h != NULL && !try_module_get(h->me))
h = NULL;
if (h != NULL && !refcount_inc_not_zero(&h->refcnt)) {
module_put(h->me);
h = NULL;
}

rcu_read_unlock();

Expand All @@ -183,6 +187,7 @@ EXPORT_SYMBOL_GPL(nf_conntrack_helper_try_module_get);

void nf_conntrack_helper_put(struct nf_conntrack_helper *helper)
{
refcount_dec(&helper->refcnt);
module_put(helper->me);
}
EXPORT_SYMBOL_GPL(nf_conntrack_helper_put);
Expand Down Expand Up @@ -423,6 +428,7 @@ int nf_conntrack_helper_register(struct nf_conntrack_helper *me)
}
}
}
refcount_set(&me->refcnt, 1);
hlist_add_head_rcu(&me->hnode, &nf_ct_helper_hash[h]);
nf_ct_helper_count++;
out:
Expand Down
17 changes: 11 additions & 6 deletions net/netfilter/nfnetlink_cthelper.c
Original file line number Diff line number Diff line change
Expand Up @@ -686,6 +686,7 @@ static int nfnl_cthelper_del(struct net *net, struct sock *nfnl,
tuple_set = true;
}

ret = -ENOENT;
list_for_each_entry_safe(nlcth, n, &nfnl_cthelper_list, list) {
cur = &nlcth->helper;
j++;
Expand All @@ -699,16 +700,20 @@ static int nfnl_cthelper_del(struct net *net, struct sock *nfnl,
tuple.dst.protonum != cur->tuple.dst.protonum))
continue;

found = true;
nf_conntrack_helper_unregister(cur);
kfree(cur->expect_policy);
if (refcount_dec_if_one(&cur->refcnt)) {
found = true;
nf_conntrack_helper_unregister(cur);
kfree(cur->expect_policy);

list_del(&nlcth->list);
kfree(nlcth);
list_del(&nlcth->list);
kfree(nlcth);
} else {
ret = -EBUSY;
}
}

/* Make sure we return success if we flush and there is no helpers */
return (found || j == 0) ? 0 : -ENOENT;
return (found || j == 0) ? 0 : ret;
}

static const struct nla_policy nfnl_cthelper_policy[NFCTH_MAX+1] = {
Expand Down

0 comments on commit 9338d7b

Please sign in to comment.