Skip to content

Commit

Permalink
Merge git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf
Browse files Browse the repository at this point in the history
Pablo Neira Ayuso says:

====================
Netfilter fixes for net

The following patchset contains Netfilter fixes for you net tree,
they are:

1) There was a race condition between parallel save/swap and delete,
   which resulted a kernel crash due to the increase ref for save, swap,
   wrong ref decrease operations. Reported and fixed by Vishwanath Pai.

2) OVS should call into CT NAT for packets of new expected connections only
   when the conntrack state is persisted with the 'commit' option to the
   OVS CT action. From Jarno Rajahalme.

3) Resolve kconfig dependencies with new OVS NAT support. From Arnd Bergmann.

4) Early validation of entry->target_offset to make sure it doesn't take us
   out from the blob, from Florian Westphal.

5) Again early validation of entry->next_offset to make sure it doesn't take
   out from the blob, also from Florian.

6) Check that entry->target_offset is always of of sizeof(struct xt_entry)
   for unconditional entries, when checking both from check_underflow()
   and when checking for loops in mark_source_chains(), again from
   Florian.

7) Fix inconsistent behaviour in nfnetlink_queue when
   NFQA_CFG_F_FAIL_OPEN is set and netlink_unicast() fails due to buffer
   overrun, we have to reinject the packet as the user expects.

8) Enforce nul-terminated table names from getsockopt GET_ENTRIES
   requests.

9) Don't assume skb->sk is set from nft_bridge_reject and synproxy,
   this fixes a recent update of the code to namespaceify
   ip_default_ttl, patch from Liping Zhang.

This batch comes with four patches to validate x_tables blobs coming
from userspace. CONFIG_USERNS exposes the x_tables interface to
unpriviledged users and to be honest this interface never received the
attention for this move away from the CAP_NET_ADMIN domain. Florian is
working on another round with more patches with more sanity checks, so
expect a bit more Netfilter fixes in this development cycle than usual.
====================

Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
David S. Miller committed Mar 28, 2016
2 parents 0e3e799 + 2942119 commit 0c84ea1
Show file tree
Hide file tree
Showing 14 changed files with 170 additions and 122 deletions.
4 changes: 4 additions & 0 deletions include/linux/netfilter/ipset/ip_set.h
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,10 @@ struct ip_set {
spinlock_t lock;
/* References to the set */
u32 ref;
/* References to the set for netlink events like dump,
* ref can be swapped out by ip_set_swap
*/
u32 ref_netlink;
/* The core set type */
struct ip_set_type *type;
/* The type variant doing the real job */
Expand Down
4 changes: 4 additions & 0 deletions net/bridge/netfilter/ebtables.c
Original file line number Diff line number Diff line change
Expand Up @@ -1521,6 +1521,8 @@ static int do_ebt_get_ctl(struct sock *sk, int cmd, void __user *user, int *len)
if (copy_from_user(&tmp, user, sizeof(tmp)))
return -EFAULT;

tmp.name[sizeof(tmp.name) - 1] = '\0';

t = find_table_lock(net, tmp.name, &ret, &ebt_mutex);
if (!t)
return ret;
Expand Down Expand Up @@ -2332,6 +2334,8 @@ static int compat_do_ebt_get_ctl(struct sock *sk, int cmd,
if (copy_from_user(&tmp, user, sizeof(tmp)))
return -EFAULT;

tmp.name[sizeof(tmp.name) - 1] = '\0';

t = find_table_lock(net, tmp.name, &ret, &ebt_mutex);
if (!t)
return ret;
Expand Down
20 changes: 10 additions & 10 deletions net/bridge/netfilter/nft_reject_bridge.c
Original file line number Diff line number Diff line change
Expand Up @@ -40,15 +40,15 @@ static void nft_reject_br_push_etherhdr(struct sk_buff *oldskb,
/* We cannot use oldskb->dev, it can be either bridge device (NF_BRIDGE INPUT)
* or the bridge port (NF_BRIDGE PREROUTING).
*/
static void nft_reject_br_send_v4_tcp_reset(struct sk_buff *oldskb,
static void nft_reject_br_send_v4_tcp_reset(struct net *net,
struct sk_buff *oldskb,
const struct net_device *dev,
int hook)
{
struct sk_buff *nskb;
struct iphdr *niph;
const struct tcphdr *oth;
struct tcphdr _oth;
struct net *net = sock_net(oldskb->sk);

if (!nft_bridge_iphdr_validate(oldskb))
return;
Expand All @@ -75,7 +75,8 @@ static void nft_reject_br_send_v4_tcp_reset(struct sk_buff *oldskb,
br_deliver(br_port_get_rcu(dev), nskb);
}

static void nft_reject_br_send_v4_unreach(struct sk_buff *oldskb,
static void nft_reject_br_send_v4_unreach(struct net *net,
struct sk_buff *oldskb,
const struct net_device *dev,
int hook, u8 code)
{
Expand All @@ -86,7 +87,6 @@ static void nft_reject_br_send_v4_unreach(struct sk_buff *oldskb,
void *payload;
__wsum csum;
u8 proto;
struct net *net = sock_net(oldskb->sk);

if (oldskb->csum_bad || !nft_bridge_iphdr_validate(oldskb))
return;
Expand Down Expand Up @@ -273,17 +273,17 @@ static void nft_reject_bridge_eval(const struct nft_expr *expr,
case htons(ETH_P_IP):
switch (priv->type) {
case NFT_REJECT_ICMP_UNREACH:
nft_reject_br_send_v4_unreach(pkt->skb, pkt->in,
pkt->hook,
nft_reject_br_send_v4_unreach(pkt->net, pkt->skb,
pkt->in, pkt->hook,
priv->icmp_code);
break;
case NFT_REJECT_TCP_RST:
nft_reject_br_send_v4_tcp_reset(pkt->skb, pkt->in,
pkt->hook);
nft_reject_br_send_v4_tcp_reset(pkt->net, pkt->skb,
pkt->in, pkt->hook);
break;
case NFT_REJECT_ICMPX_UNREACH:
nft_reject_br_send_v4_unreach(pkt->skb, pkt->in,
pkt->hook,
nft_reject_br_send_v4_unreach(pkt->net, pkt->skb,
pkt->in, pkt->hook,
nft_reject_icmp_code(priv->icmp_code));
break;
}
Expand Down
43 changes: 23 additions & 20 deletions net/ipv4/netfilter/arp_tables.c
Original file line number Diff line number Diff line change
Expand Up @@ -359,11 +359,12 @@ unsigned int arpt_do_table(struct sk_buff *skb,
}

/* All zeroes == unconditional rule. */
static inline bool unconditional(const struct arpt_arp *arp)
static inline bool unconditional(const struct arpt_entry *e)
{
static const struct arpt_arp uncond;

return memcmp(arp, &uncond, sizeof(uncond)) == 0;
return e->target_offset == sizeof(struct arpt_entry) &&
memcmp(&e->arp, &uncond, sizeof(uncond)) == 0;
}

/* Figures out from what hook each rule can be called: returns 0 if
Expand Down Expand Up @@ -402,11 +403,10 @@ static int mark_source_chains(const struct xt_table_info *newinfo,
|= ((1 << hook) | (1 << NF_ARP_NUMHOOKS));

/* Unconditional return/END. */
if ((e->target_offset == sizeof(struct arpt_entry) &&
if ((unconditional(e) &&
(strcmp(t->target.u.user.name,
XT_STANDARD_TARGET) == 0) &&
t->verdict < 0 && unconditional(&e->arp)) ||
visited) {
t->verdict < 0) || visited) {
unsigned int oldpos, size;

if ((strcmp(t->target.u.user.name,
Expand Down Expand Up @@ -474,14 +474,12 @@ static int mark_source_chains(const struct xt_table_info *newinfo,
return 1;
}

static inline int check_entry(const struct arpt_entry *e, const char *name)
static inline int check_entry(const struct arpt_entry *e)
{
const struct xt_entry_target *t;

if (!arp_checkentry(&e->arp)) {
duprintf("arp_tables: arp check failed %p %s.\n", e, name);
if (!arp_checkentry(&e->arp))
return -EINVAL;
}

if (e->target_offset + sizeof(struct xt_entry_target) > e->next_offset)
return -EINVAL;
Expand Down Expand Up @@ -522,10 +520,6 @@ find_check_entry(struct arpt_entry *e, const char *name, unsigned int size)
struct xt_target *target;
int ret;

ret = check_entry(e, name);
if (ret)
return ret;

e->counters.pcnt = xt_percpu_counter_alloc();
if (IS_ERR_VALUE(e->counters.pcnt))
return -ENOMEM;
Expand Down Expand Up @@ -557,7 +551,7 @@ static bool check_underflow(const struct arpt_entry *e)
const struct xt_entry_target *t;
unsigned int verdict;

if (!unconditional(&e->arp))
if (!unconditional(e))
return false;
t = arpt_get_target_c(e);
if (strcmp(t->u.user.name, XT_STANDARD_TARGET) != 0)
Expand All @@ -576,9 +570,11 @@ static inline int check_entry_size_and_hooks(struct arpt_entry *e,
unsigned int valid_hooks)
{
unsigned int h;
int err;

if ((unsigned long)e % __alignof__(struct arpt_entry) != 0 ||
(unsigned char *)e + sizeof(struct arpt_entry) >= limit) {
(unsigned char *)e + sizeof(struct arpt_entry) >= limit ||
(unsigned char *)e + e->next_offset > limit) {
duprintf("Bad offset %p\n", e);
return -EINVAL;
}
Expand All @@ -590,6 +586,10 @@ static inline int check_entry_size_and_hooks(struct arpt_entry *e,
return -EINVAL;
}

err = check_entry(e);
if (err)
return err;

/* Check hooks & underflows */
for (h = 0; h < NF_ARP_NUMHOOKS; h++) {
if (!(valid_hooks & (1 << h)))
Expand All @@ -598,9 +598,9 @@ static inline int check_entry_size_and_hooks(struct arpt_entry *e,
newinfo->hook_entry[h] = hook_entries[h];
if ((unsigned char *)e - base == underflows[h]) {
if (!check_underflow(e)) {
pr_err("Underflows must be unconditional and "
"use the STANDARD target with "
"ACCEPT/DROP\n");
pr_debug("Underflows must be unconditional and "
"use the STANDARD target with "
"ACCEPT/DROP\n");
return -EINVAL;
}
newinfo->underflow[h] = underflows[h];
Expand Down Expand Up @@ -969,6 +969,7 @@ static int get_entries(struct net *net, struct arpt_get_entries __user *uptr,
sizeof(struct arpt_get_entries) + get.size);
return -EINVAL;
}
get.name[sizeof(get.name) - 1] = '\0';

t = xt_find_table_lock(net, NFPROTO_ARP, get.name);
if (!IS_ERR_OR_NULL(t)) {
Expand Down Expand Up @@ -1233,7 +1234,8 @@ check_compat_entry_size_and_hooks(struct compat_arpt_entry *e,

duprintf("check_compat_entry_size_and_hooks %p\n", e);
if ((unsigned long)e % __alignof__(struct compat_arpt_entry) != 0 ||
(unsigned char *)e + sizeof(struct compat_arpt_entry) >= limit) {
(unsigned char *)e + sizeof(struct compat_arpt_entry) >= limit ||
(unsigned char *)e + e->next_offset > limit) {
duprintf("Bad offset %p, limit = %p\n", e, limit);
return -EINVAL;
}
Expand All @@ -1246,7 +1248,7 @@ check_compat_entry_size_and_hooks(struct compat_arpt_entry *e,
}

/* For purposes of check_entry casting the compat entry is fine */
ret = check_entry((struct arpt_entry *)e, name);
ret = check_entry((struct arpt_entry *)e);
if (ret)
return ret;

Expand Down Expand Up @@ -1662,6 +1664,7 @@ static int compat_get_entries(struct net *net,
*len, sizeof(get) + get.size);
return -EINVAL;
}
get.name[sizeof(get.name) - 1] = '\0';

xt_compat_lock(NFPROTO_ARP);
t = xt_find_table_lock(net, NFPROTO_ARP, get.name);
Expand Down
48 changes: 25 additions & 23 deletions net/ipv4/netfilter/ip_tables.c
Original file line number Diff line number Diff line change
Expand Up @@ -168,11 +168,12 @@ get_entry(const void *base, unsigned int offset)

/* All zeroes == unconditional rule. */
/* Mildly perf critical (only if packet tracing is on) */
static inline bool unconditional(const struct ipt_ip *ip)
static inline bool unconditional(const struct ipt_entry *e)
{
static const struct ipt_ip uncond;

return memcmp(ip, &uncond, sizeof(uncond)) == 0;
return e->target_offset == sizeof(struct ipt_entry) &&
memcmp(&e->ip, &uncond, sizeof(uncond)) == 0;
#undef FWINV
}

Expand Down Expand Up @@ -229,11 +230,10 @@ get_chainname_rulenum(const struct ipt_entry *s, const struct ipt_entry *e,
} else if (s == e) {
(*rulenum)++;

if (s->target_offset == sizeof(struct ipt_entry) &&
if (unconditional(s) &&
strcmp(t->target.u.kernel.target->name,
XT_STANDARD_TARGET) == 0 &&
t->verdict < 0 &&
unconditional(&s->ip)) {
t->verdict < 0) {
/* Tail of chains: STANDARD target (return/policy) */
*comment = *chainname == hookname
? comments[NF_IP_TRACE_COMMENT_POLICY]
Expand Down Expand Up @@ -476,11 +476,10 @@ mark_source_chains(const struct xt_table_info *newinfo,
e->comefrom |= ((1 << hook) | (1 << NF_INET_NUMHOOKS));

/* Unconditional return/END. */
if ((e->target_offset == sizeof(struct ipt_entry) &&
if ((unconditional(e) &&
(strcmp(t->target.u.user.name,
XT_STANDARD_TARGET) == 0) &&
t->verdict < 0 && unconditional(&e->ip)) ||
visited) {
t->verdict < 0) || visited) {
unsigned int oldpos, size;

if ((strcmp(t->target.u.user.name,
Expand Down Expand Up @@ -569,14 +568,12 @@ static void cleanup_match(struct xt_entry_match *m, struct net *net)
}

static int
check_entry(const struct ipt_entry *e, const char *name)
check_entry(const struct ipt_entry *e)
{
const struct xt_entry_target *t;

if (!ip_checkentry(&e->ip)) {
duprintf("ip check failed %p %s.\n", e, name);
if (!ip_checkentry(&e->ip))
return -EINVAL;
}

if (e->target_offset + sizeof(struct xt_entry_target) >
e->next_offset)
Expand Down Expand Up @@ -666,10 +663,6 @@ find_check_entry(struct ipt_entry *e, struct net *net, const char *name,
struct xt_mtchk_param mtpar;
struct xt_entry_match *ematch;

ret = check_entry(e, name);
if (ret)
return ret;

e->counters.pcnt = xt_percpu_counter_alloc();
if (IS_ERR_VALUE(e->counters.pcnt))
return -ENOMEM;
Expand Down Expand Up @@ -721,7 +714,7 @@ static bool check_underflow(const struct ipt_entry *e)
const struct xt_entry_target *t;
unsigned int verdict;

if (!unconditional(&e->ip))
if (!unconditional(e))
return false;
t = ipt_get_target_c(e);
if (strcmp(t->u.user.name, XT_STANDARD_TARGET) != 0)
Expand All @@ -741,9 +734,11 @@ check_entry_size_and_hooks(struct ipt_entry *e,
unsigned int valid_hooks)
{
unsigned int h;
int err;

if ((unsigned long)e % __alignof__(struct ipt_entry) != 0 ||
(unsigned char *)e + sizeof(struct ipt_entry) >= limit) {
(unsigned char *)e + sizeof(struct ipt_entry) >= limit ||
(unsigned char *)e + e->next_offset > limit) {
duprintf("Bad offset %p\n", e);
return -EINVAL;
}
Expand All @@ -755,6 +750,10 @@ check_entry_size_and_hooks(struct ipt_entry *e,
return -EINVAL;
}

err = check_entry(e);
if (err)
return err;

/* Check hooks & underflows */
for (h = 0; h < NF_INET_NUMHOOKS; h++) {
if (!(valid_hooks & (1 << h)))
Expand All @@ -763,9 +762,9 @@ check_entry_size_and_hooks(struct ipt_entry *e,
newinfo->hook_entry[h] = hook_entries[h];
if ((unsigned char *)e - base == underflows[h]) {
if (!check_underflow(e)) {
pr_err("Underflows must be unconditional and "
"use the STANDARD target with "
"ACCEPT/DROP\n");
pr_debug("Underflows must be unconditional and "
"use the STANDARD target with "
"ACCEPT/DROP\n");
return -EINVAL;
}
newinfo->underflow[h] = underflows[h];
Expand Down Expand Up @@ -1157,6 +1156,7 @@ get_entries(struct net *net, struct ipt_get_entries __user *uptr,
*len, sizeof(get) + get.size);
return -EINVAL;
}
get.name[sizeof(get.name) - 1] = '\0';

t = xt_find_table_lock(net, AF_INET, get.name);
if (!IS_ERR_OR_NULL(t)) {
Expand Down Expand Up @@ -1493,7 +1493,8 @@ check_compat_entry_size_and_hooks(struct compat_ipt_entry *e,

duprintf("check_compat_entry_size_and_hooks %p\n", e);
if ((unsigned long)e % __alignof__(struct compat_ipt_entry) != 0 ||
(unsigned char *)e + sizeof(struct compat_ipt_entry) >= limit) {
(unsigned char *)e + sizeof(struct compat_ipt_entry) >= limit ||
(unsigned char *)e + e->next_offset > limit) {
duprintf("Bad offset %p, limit = %p\n", e, limit);
return -EINVAL;
}
Expand All @@ -1506,7 +1507,7 @@ check_compat_entry_size_and_hooks(struct compat_ipt_entry *e,
}

/* For purposes of check_entry casting the compat entry is fine */
ret = check_entry((struct ipt_entry *)e, name);
ret = check_entry((struct ipt_entry *)e);
if (ret)
return ret;

Expand Down Expand Up @@ -1935,6 +1936,7 @@ compat_get_entries(struct net *net, struct compat_ipt_get_entries __user *uptr,
*len, sizeof(get) + get.size);
return -EINVAL;
}
get.name[sizeof(get.name) - 1] = '\0';

xt_compat_lock(AF_INET);
t = xt_find_table_lock(net, AF_INET, get.name);
Expand Down
Loading

0 comments on commit 0c84ea1

Please sign in to comment.