[PATCH 0/2] Netfilter fixes for net-next

[PATCH 0/2] Netfilter fixes for net-next

[Pablo Neira Ayuso]

Hi David,

This batch contains two fixes for what you have in your net-next,
they are:

1) Remove nf_send_reset6() from header file. This function now resides
   in the nf_reject_ipv6 module. Reported by Eric Dumazet.

2) Fix wrong NFT_REJECT_ICMPX_MAX definition and adjust code to fix
   errors reported by Dan Carpenter's static analysis tools.

You can pull these changes from:

  git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf-next.git

Thanks!

----------------------------------------------------------------

The following changes since commit 4e62ccd901062c532673f4fda16c484de2c3c8fc:

  Merge branch 'mlx4-next' (2014-10-06 01:04:21 -0400)

are available in the git repository at:


  git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf-next.git master

for you to fetch changes up to f0d1f04f0a2f662b6b617e24d115fddcf6ef8723:

  netfilter: fix wrong arithmetics regarding NFT_REJECT_ICMPX_MAX (2014-10-07 20:16:31 +0200)

----------------------------------------------------------------
Pablo Neira Ayuso (2):
      netfilter: kill nf_send_reset6() from include/net/netfilter/ipv6/nf_reject.h
      netfilter: fix wrong arithmetics regarding NFT_REJECT_ICMPX_MAX

 include/net/netfilter/ipv6/nf_reject.h   |  157 +-----------------------------
 include/uapi/linux/netfilter/nf_tables.h |    2 +-
 net/netfilter/nft_reject.c               |   10 +-
 3 files changed, 7 insertions(+), 162 deletions(-)

[PATCH 1/2] netfilter: kill nf_send_reset6() from include/net/netfilter/ipv6/nf_reject.h

[Pablo Neira Ayuso]

nf_send_reset6() now resides in net/ipv6/netfilter/nf_reject_ipv6.c

Fixes: c8d7b98 ("netfilter: move nf_send_resetX() code to nf_reject_ipvX modules")
Reported-by: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Acked-by: Eric Dumazet <edumazet@google.com>
---
 include/net/netfilter/ipv6/nf_reject.h |  157 +-------------------------------
 1 file changed, 2 insertions(+), 155 deletions(-)

diff --git a/include/net/netfilter/ipv6/nf_reject.h b/include/net/netfilter/ipv6/nf_reject.h
index 7a10cfc..48e1881 100644
--- a/include/net/netfilter/ipv6/nf_reject.h
+++ b/include/net/netfilter/ipv6/nf_reject.h
@@ -1,11 +1,7 @@
 #ifndef _IPV6_NF_REJECT_H
 #define _IPV6_NF_REJECT_H
 
-#include <net/ipv6.h>
-#include <net/ip6_route.h>
-#include <net/ip6_fib.h>
-#include <net/ip6_checksum.h>
-#include <linux/netfilter_ipv6.h>
+#include <linux/icmpv6.h>
 
 static inline void
 nf_send_unreach6(struct net *net, struct sk_buff *skb_in, unsigned char code,
@@ -17,155 +13,6 @@ nf_send_unreach6(struct net *net, struct sk_buff *skb_in, unsigned char code,
 	icmpv6_send(skb_in, ICMPV6_DEST_UNREACH, code, 0);
 }
 
-/* Send RST reply */
-static void nf_send_reset6(struct net *net, struct sk_buff *oldskb, int hook)
-{
-	struct sk_buff *nskb;
-	struct tcphdr otcph, *tcph;
-	unsigned int otcplen, hh_len;
-	int tcphoff, needs_ack;
-	const struct ipv6hdr *oip6h = ipv6_hdr(oldskb);
-	struct ipv6hdr *ip6h;
-#define DEFAULT_TOS_VALUE	0x0U
-	const __u8 tclass = DEFAULT_TOS_VALUE;
-	struct dst_entry *dst = NULL;
-	u8 proto;
-	__be16 frag_off;
-	struct flowi6 fl6;
-
-	if ((!(ipv6_addr_type(&oip6h->saddr) & IPV6_ADDR_UNICAST)) ||
-	    (!(ipv6_addr_type(&oip6h->daddr) & IPV6_ADDR_UNICAST))) {
-		pr_debug("addr is not unicast.\n");
-		return;
-	}
-
-	proto = oip6h->nexthdr;
-	tcphoff = ipv6_skip_exthdr(oldskb, ((u8*)(oip6h+1) - oldskb->data), &proto, &frag_off);
-
-	if ((tcphoff < 0) || (tcphoff > oldskb->len)) {
-		pr_debug("Cannot get TCP header.\n");
-		return;
-	}
-
-	otcplen = oldskb->len - tcphoff;
-
-	/* IP header checks: fragment, too short. */
-	if (proto != IPPROTO_TCP || otcplen < sizeof(struct tcphdr)) {
-		pr_debug("proto(%d) != IPPROTO_TCP, "
-			 "or too short. otcplen = %d\n",
-			 proto, otcplen);
-		return;
-	}
-
-	if (skb_copy_bits(oldskb, tcphoff, &otcph, sizeof(struct tcphdr)))
-		BUG();
-
-	/* No RST for RST. */
-	if (otcph.rst) {
-		pr_debug("RST is set\n");
-		return;
-	}
-
-	/* Check checksum. */
-	if (nf_ip6_checksum(oldskb, hook, tcphoff, IPPROTO_TCP)) {
-		pr_debug("TCP checksum is invalid\n");
-		return;
-	}
-
-	memset(&fl6, 0, sizeof(fl6));
-	fl6.flowi6_proto = IPPROTO_TCP;
-	fl6.saddr = oip6h->daddr;
-	fl6.daddr = oip6h->saddr;
-	fl6.fl6_sport = otcph.dest;
-	fl6.fl6_dport = otcph.source;
-	security_skb_classify_flow(oldskb, flowi6_to_flowi(&fl6));
-	dst = ip6_route_output(net, NULL, &fl6);
-	if (dst == NULL || dst->error) {
-		dst_release(dst);
-		return;
-	}
-	dst = xfrm_lookup(net, dst, flowi6_to_flowi(&fl6), NULL, 0);
-	if (IS_ERR(dst))
-		return;
-
-	hh_len = (dst->dev->hard_header_len + 15)&~15;
-	nskb = alloc_skb(hh_len + 15 + dst->header_len + sizeof(struct ipv6hdr)
-			 + sizeof(struct tcphdr) + dst->trailer_len,
-			 GFP_ATOMIC);
-
-	if (!nskb) {
-		net_dbg_ratelimited("cannot alloc skb\n");
-		dst_release(dst);
-		return;
-	}
-
-	skb_dst_set(nskb, dst);
-
-	skb_reserve(nskb, hh_len + dst->header_len);
-
-	skb_put(nskb, sizeof(struct ipv6hdr));
-	skb_reset_network_header(nskb);
-	ip6h = ipv6_hdr(nskb);
-	ip6_flow_hdr(ip6h, tclass, 0);
-	ip6h->hop_limit = ip6_dst_hoplimit(dst);
-	ip6h->nexthdr = IPPROTO_TCP;
-	ip6h->saddr = oip6h->daddr;
-	ip6h->daddr = oip6h->saddr;
-
-	skb_reset_transport_header(nskb);
-	tcph = (struct tcphdr *)skb_put(nskb, sizeof(struct tcphdr));
-	/* Truncate to length (no data) */
-	tcph->doff = sizeof(struct tcphdr)/4;
-	tcph->source = otcph.dest;
-	tcph->dest = otcph.source;
-
-	if (otcph.ack) {
-		needs_ack = 0;
-		tcph->seq = otcph.ack_seq;
-		tcph->ack_seq = 0;
-	} else {
-		needs_ack = 1;
-		tcph->ack_seq = htonl(ntohl(otcph.seq) + otcph.syn + otcph.fin
-				      + otcplen - (otcph.doff<<2));
-		tcph->seq = 0;
-	}
-
-	/* Reset flags */
-	((u_int8_t *)tcph)[13] = 0;
-	tcph->rst = 1;
-	tcph->ack = needs_ack;
-	tcph->window = 0;
-	tcph->urg_ptr = 0;
-	tcph->check = 0;
-
-	/* Adjust TCP checksum */
-	tcph->check = csum_ipv6_magic(&ipv6_hdr(nskb)->saddr,
-				      &ipv6_hdr(nskb)->daddr,
-				      sizeof(struct tcphdr), IPPROTO_TCP,
-				      csum_partial(tcph,
-						   sizeof(struct tcphdr), 0));
-
-	nf_ct_attach(nskb, oldskb);
-
-#if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)
-	/* If we use ip6_local_out for bridged traffic, the MAC source on
-	 * the RST will be ours, instead of the destination's.  This confuses
-	 * some routers/firewalls, and they drop the packet.  So we need to
-	 * build the eth header using the original destination's MAC as the
-	 * source, and send the RST packet directly.
-	 */
-	if (oldskb->nf_bridge) {
-		struct ethhdr *oeth = eth_hdr(oldskb);
-		nskb->dev = oldskb->nf_bridge->physindev;
-		nskb->protocol = htons(ETH_P_IPV6);
-		ip6h->payload_len = htons(sizeof(struct tcphdr));
-		if (dev_hard_header(nskb, nskb->dev, ntohs(nskb->protocol),
-				    oeth->h_source, oeth->h_dest, nskb->len) < 0)
-			return;
-		dev_queue_xmit(nskb);
-	} else
-#endif
-		ip6_local_out(nskb);
-}
+void nf_send_reset6(struct net *net, struct sk_buff *oldskb, int hook);
 
 #endif /* _IPV6_NF_REJECT_H */
-- 
1.7.10.4

Re: [PATCH 1/2] netfilter: kill nf_send_reset6() from include/net/netfilter/ipv6/nf_reject.h

[Josh Boyer]

[-- Attachment #1: Type: text/plain, Size: 2111 bytes --]

On Thu, Oct 9, 2014 at 2:27 PM, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> nf_send_reset6() now resides in net/ipv6/netfilter/nf_reject_ipv6.c
>
> Fixes: c8d7b98 ("netfilter: move nf_send_resetX() code to nf_reject_ipvX modules")
> Reported-by: Eric Dumazet <eric.dumazet@gmail.com>
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> Acked-by: Eric Dumazet <edumazet@google.com>
> ---
>  include/net/netfilter/ipv6/nf_reject.h |  157 +-------------------------------
>  1 file changed, 2 insertions(+), 155 deletions(-)

Hi All,

This morning I was testing a kernel build from Linus' tree as of Linux
v3.17-7639-g90eac7eee2f4.  When I rebooted my test machines, I
couldn't ssh back into any of them.  I poked around a bit and noticed
that it seems the iptables rules weren't getting loaded properly.
Traffic out worked fine, and I could ping the machine, but other
incoming traffic was blocked.  Then I saw that the ip6t_REJECT and
ip6t_rpfilter modules were not being loaded on the bad kernel.
Looking in dmesg I see:

[   14.619028] nf_reject_ipv6: module license 'unspecified' taints kernel.
[   14.619125] nf_reject_ipv6: Unknown symbol ip6_local_out (err 0)

So I did a git bisect and it pointed to this patch:

[jwboyer@obiwan linux]$ git bisect bad
91c1a09b33c902e20e09d9742560cc238a714de5 is the first bad commit
commit 91c1a09b33c902e20e09d9742560cc238a714de5
Author: Pablo Neira Ayuso <pablo@netfilter.org>
Date:   Tue Oct 7 18:48:12 2014 +0200

    netfilter: kill nf_send_reset6() from include/net/netfilter/ipv6/nf_reject.h

    nf_send_reset6() now resides in net/ipv6/netfilter/nf_reject_ipv6.c

    Fixes: c8d7b98 ("netfilter: move nf_send_resetX() code to
nf_reject_ipvX modules")
    Reported-by: Eric Dumazet <eric.dumazet@gmail.com>
    Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
    Acked-by: Eric Dumazet <edumazet@google.com>

:040000 040000 ab5b61e7ba562e0b22d781a4322ed73c657878dc
71822a7e785c408dd55a6c4600144573de500512 M      include
[jwboyer@obiwan linux]$

I've attached the bisect log.

Perhaps one too many header files were trimmed in this case?

josh

[-- Attachment #2: BISECT_LOG --]
[-- Type: application/octet-stream, Size: 2277 bytes --]

git bisect start
# bad: [90eac7eee2f4257644dcfb9d22348fded7c24afd] Merge tag 'ftracetest-3.18' of git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace
git bisect bad 90eac7eee2f4257644dcfb9d22348fded7c24afd
# good: [c798360cd1438090d51eeaa8e67985da11362eba] Merge branch 'for-3.18' of git://git.kernel.org/pub/scm/linux/kernel/git/tj/percpu
git bisect good c798360cd1438090d51eeaa8e67985da11362eba
# good: [a2ce35273c2f1aa0dcddd8822681d64ee5f31852] Merge tag 'sound-3.18-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound
git bisect good a2ce35273c2f1aa0dcddd8822681d64ee5f31852
# good: [90d0c376f5ee1927327b267faf15bf970476f09e] Merge branch 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/mason/linux-btrfs
git bisect good 90d0c376f5ee1927327b267faf15bf970476f09e
# good: [d53ba6b3bba33432cc37b7101a86f8f3392c46e7] cxl: Fix afu_read() not doing finish_wait() on signal or non-blocking
git bisect good d53ba6b3bba33432cc37b7101a86f8f3392c46e7
# good: [052db7ec86dff26f734031c3ef5c2c03a94af0af] Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/sparc
git bisect good 052db7ec86dff26f734031c3ef5c2c03a94af0af
# bad: [2403077d47991a8385789779ee5fc90b003f9fbe] Merge branch 'xgene'
git bisect bad 2403077d47991a8385789779ee5fc90b003f9fbe
# good: [b71b12dce200e4709bd9f709e71c84dcb2cf8a82] networking: fm10k: Fix build failure
git bisect good b71b12dce200e4709bd9f709e71c84dcb2cf8a82
# good: [4511a4a50e1a8757f771681c3e92dbf5a928eeac] Merge tag 'master-2014-10-08' of git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless-next
git bisect good 4511a4a50e1a8757f771681c3e92dbf5a928eeac
# bad: [b61d18904e2a99ed16b6e97d5419f1db19e08bd2] MAINTAINERS: Update APM X-Gene section
git bisect bad b61d18904e2a99ed16b6e97d5419f1db19e08bd2
# bad: [f0d1f04f0a2f662b6b617e24d115fddcf6ef8723] netfilter: fix wrong arithmetics regarding NFT_REJECT_ICMPX_MAX
git bisect bad f0d1f04f0a2f662b6b617e24d115fddcf6ef8723
# bad: [91c1a09b33c902e20e09d9742560cc238a714de5] netfilter: kill nf_send_reset6() from include/net/netfilter/ipv6/nf_reject.h
git bisect bad 91c1a09b33c902e20e09d9742560cc238a714de5
# first bad commit: [91c1a09b33c902e20e09d9742560cc238a714de5] netfilter: kill nf_send_reset6() from include/net/netfilter/ipv6/nf_reject.h

Re: [PATCH 1/2] netfilter: kill nf_send_reset6() from include/net/netfilter/ipv6/nf_reject.h

[Florian Westphal]

Josh Boyer <jwboyer@fedoraproject.org> wrote:
> On Thu, Oct 9, 2014 at 2:27 PM, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > nf_send_reset6() now resides in net/ipv6/netfilter/nf_reject_ipv6.c
> >
> > Fixes: c8d7b98 ("netfilter: move nf_send_resetX() code to nf_reject_ipvX modules")
> > Reported-by: Eric Dumazet <eric.dumazet@gmail.com>
> > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> > Acked-by: Eric Dumazet <edumazet@google.com>
> > ---
> >  include/net/netfilter/ipv6/nf_reject.h |  157 +-------------------------------
> >  1 file changed, 2 insertions(+), 155 deletions(-)
> 
> Hi All,
> 
> This morning I was testing a kernel build from Linus' tree as of Linux
> v3.17-7639-g90eac7eee2f4.  When I rebooted my test machines, I
> couldn't ssh back into any of them.  I poked around a bit and noticed
> that it seems the iptables rules weren't getting loaded properly.
> Traffic out worked fine, and I could ping the machine, but other
> incoming traffic was blocked.  Then I saw that the ip6t_REJECT and
> ip6t_rpfilter modules were not being loaded on the bad kernel.
> Looking in dmesg I see:
> 
> [   14.619028] nf_reject_ipv6: module license 'unspecified' taints kernel.
> [   14.619125] nf_reject_ipv6: Unknown symbol ip6_local_out (err 0)

Ouch. ip6_local_is EXPORT_SYMBOL_GPL.

http://patchwork.ozlabs.org/patch/398501/

should fix this.

Re: [PATCH 1/2] netfilter: kill nf_send_reset6() from include/net/netfilter/ipv6/nf_reject.h

[Josh Boyer]

On Mon, Oct 13, 2014 at 11:55 AM, Florian Westphal <fw@strlen.de> wrote:
> Josh Boyer <jwboyer@fedoraproject.org> wrote:
>> On Thu, Oct 9, 2014 at 2:27 PM, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>> > nf_send_reset6() now resides in net/ipv6/netfilter/nf_reject_ipv6.c
>> >
>> > Fixes: c8d7b98 ("netfilter: move nf_send_resetX() code to nf_reject_ipvX modules")
>> > Reported-by: Eric Dumazet <eric.dumazet@gmail.com>
>> > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
>> > Acked-by: Eric Dumazet <edumazet@google.com>
>> > ---
>> >  include/net/netfilter/ipv6/nf_reject.h |  157 +-------------------------------
>> >  1 file changed, 2 insertions(+), 155 deletions(-)
>>
>> Hi All,
>>
>> This morning I was testing a kernel build from Linus' tree as of Linux
>> v3.17-7639-g90eac7eee2f4.  When I rebooted my test machines, I
>> couldn't ssh back into any of them.  I poked around a bit and noticed
>> that it seems the iptables rules weren't getting loaded properly.
>> Traffic out worked fine, and I could ping the machine, but other
>> incoming traffic was blocked.  Then I saw that the ip6t_REJECT and
>> ip6t_rpfilter modules were not being loaded on the bad kernel.
>> Looking in dmesg I see:
>>
>> [   14.619028] nf_reject_ipv6: module license 'unspecified' taints kernel.
>> [   14.619125] nf_reject_ipv6: Unknown symbol ip6_local_out (err 0)
>
> Ouch. ip6_local_is EXPORT_SYMBOL_GPL.
>
> http://patchwork.ozlabs.org/patch/398501/
>
> should fix this.

I believe you're correct.  I dug in some more and I was just about to
send a similar patch.  I'll add it on top of my builds and test it
out.  Thanks for the pointer.

josh

Re: [PATCH 1/2] netfilter: kill nf_send_reset6() from include/net/netfilter/ipv6/nf_reject.h

[Josh Boyer]

On Mon, Oct 13, 2014 at 12:01 PM, Josh Boyer <jwboyer@fedoraproject.org> wrote:
> On Mon, Oct 13, 2014 at 11:55 AM, Florian Westphal <fw@strlen.de> wrote:
>> Josh Boyer <jwboyer@fedoraproject.org> wrote:
>>> On Thu, Oct 9, 2014 at 2:27 PM, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>>> > nf_send_reset6() now resides in net/ipv6/netfilter/nf_reject_ipv6.c
>>> >
>>> > Fixes: c8d7b98 ("netfilter: move nf_send_resetX() code to nf_reject_ipvX modules")
>>> > Reported-by: Eric Dumazet <eric.dumazet@gmail.com>
>>> > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
>>> > Acked-by: Eric Dumazet <edumazet@google.com>
>>> > ---
>>> >  include/net/netfilter/ipv6/nf_reject.h |  157 +-------------------------------
>>> >  1 file changed, 2 insertions(+), 155 deletions(-)
>>>
>>> Hi All,
>>>
>>> This morning I was testing a kernel build from Linus' tree as of Linux
>>> v3.17-7639-g90eac7eee2f4.  When I rebooted my test machines, I
>>> couldn't ssh back into any of them.  I poked around a bit and noticed
>>> that it seems the iptables rules weren't getting loaded properly.
>>> Traffic out worked fine, and I could ping the machine, but other
>>> incoming traffic was blocked.  Then I saw that the ip6t_REJECT and
>>> ip6t_rpfilter modules were not being loaded on the bad kernel.
>>> Looking in dmesg I see:
>>>
>>> [   14.619028] nf_reject_ipv6: module license 'unspecified' taints kernel.
>>> [   14.619125] nf_reject_ipv6: Unknown symbol ip6_local_out (err 0)
>>
>> Ouch. ip6_local_is EXPORT_SYMBOL_GPL.
>>
>> http://patchwork.ozlabs.org/patch/398501/
>>
>> should fix this.
>
> I believe you're correct.  I dug in some more and I was just about to
> send a similar patch.  I'll add it on top of my builds and test it
> out.  Thanks for the pointer.

The patch you pointed to does indeed fix the issues I was seeing.
Thank you very much for the fast response.

josh

[PATCH 2/2] netfilter: fix wrong arithmetics regarding NFT_REJECT_ICMPX_MAX

[Pablo Neira Ayuso]

NFT_REJECT_ICMPX_MAX should be __NFT_REJECT_ICMPX_MAX - 1.

nft_reject_icmp_code() and nft_reject_icmpv6_code() are called from the
packet path, so BUG_ON in case we try to access an unknown abstracted
ICMP code. This should not happen since we already validate this from
nft_reject_{inet,bridge}_init().

Fixes: 51b0a5d ("netfilter: nft_reject: introduce icmp code abstraction for inet and bridge")
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/uapi/linux/netfilter/nf_tables.h |    2 +-
 net/netfilter/nft_reject.c               |   10 ++++------
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
index c26df67..f31fe7b 100644
--- a/include/uapi/linux/netfilter/nf_tables.h
+++ b/include/uapi/linux/netfilter/nf_tables.h
@@ -774,7 +774,7 @@ enum nft_reject_inet_code {
 	NFT_REJECT_ICMPX_ADMIN_PROHIBITED,
 	__NFT_REJECT_ICMPX_MAX
 };
-#define NFT_REJECT_ICMPX_MAX	(__NFT_REJECT_ICMPX_MAX + 1)
+#define NFT_REJECT_ICMPX_MAX	(__NFT_REJECT_ICMPX_MAX - 1)
 
 /**
  * enum nft_reject_attributes - nf_tables reject expression netlink attributes
diff --git a/net/netfilter/nft_reject.c b/net/netfilter/nft_reject.c
index ec8a456..57d3e1a 100644
--- a/net/netfilter/nft_reject.c
+++ b/net/netfilter/nft_reject.c
@@ -72,7 +72,7 @@ nla_put_failure:
 }
 EXPORT_SYMBOL_GPL(nft_reject_dump);
 
-static u8 icmp_code_v4[NFT_REJECT_ICMPX_MAX] = {
+static u8 icmp_code_v4[NFT_REJECT_ICMPX_MAX + 1] = {
 	[NFT_REJECT_ICMPX_NO_ROUTE]		= ICMP_NET_UNREACH,
 	[NFT_REJECT_ICMPX_PORT_UNREACH]		= ICMP_PORT_UNREACH,
 	[NFT_REJECT_ICMPX_HOST_UNREACH]		= ICMP_HOST_UNREACH,
@@ -81,8 +81,7 @@ static u8 icmp_code_v4[NFT_REJECT_ICMPX_MAX] = {
 
 int nft_reject_icmp_code(u8 code)
 {
-	if (code > NFT_REJECT_ICMPX_MAX)
-		return -EINVAL;
+	BUG_ON(code > NFT_REJECT_ICMPX_MAX);
 
 	return icmp_code_v4[code];
 }
@@ -90,7 +89,7 @@ int nft_reject_icmp_code(u8 code)
 EXPORT_SYMBOL_GPL(nft_reject_icmp_code);
 
 
-static u8 icmp_code_v6[NFT_REJECT_ICMPX_MAX] = {
+static u8 icmp_code_v6[NFT_REJECT_ICMPX_MAX + 1] = {
 	[NFT_REJECT_ICMPX_NO_ROUTE]		= ICMPV6_NOROUTE,
 	[NFT_REJECT_ICMPX_PORT_UNREACH]		= ICMPV6_PORT_UNREACH,
 	[NFT_REJECT_ICMPX_HOST_UNREACH]		= ICMPV6_ADDR_UNREACH,
@@ -99,8 +98,7 @@ static u8 icmp_code_v6[NFT_REJECT_ICMPX_MAX] = {
 
 int nft_reject_icmpv6_code(u8 code)
 {
-	if (code > NFT_REJECT_ICMPX_MAX)
-		return -EINVAL;
+	BUG_ON(code > NFT_REJECT_ICMPX_MAX);
 
 	return icmp_code_v6[code];
 }
-- 
1.7.10.4

Re: [PATCH 0/2] Netfilter fixes for net-next

[David Miller]

From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Thu,  9 Oct 2014 20:27:39 +0200

> This batch contains two fixes for what you have in your net-next,
> they are:
> 
> 1) Remove nf_send_reset6() from header file. This function now resides
>    in the nf_reject_ipv6 module. Reported by Eric Dumazet.
> 
> 2) Fix wrong NFT_REJECT_ICMPX_MAX definition and adjust code to fix
>    errors reported by Dan Carpenter's static analysis tools.
> 
> You can pull these changes from:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf-next.git

Pulled, thanks Pablo.

[PATCH 2/2] netfilter: fix wrong arithmetics regarding NFT_REJECT_ICMPX_MAX

[Pablo Neira Ayuso]

NFT_REJECT_ICMPX_MAX should be __NFT_REJECT_ICMPX_MAX - 1.

nft_reject_icmp_code() and nft_reject_icmpv6_code() are called from the
packet path, so BUG_ON in case we try to access an unknown abstracted
ICMP code. This should not happen since we already validate this from
the nft_reject_{inet,bridge}_init.

Fixes: 51b0a5d ("netfilter: nft_reject: introduce icmp code abstraction for inet and bridge")
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/uapi/linux/netfilter/nf_tables.h |    2 +-
 net/netfilter/nft_reject.c               |   10 ++++------
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
index c26df67..f31fe7b 100644
--- a/include/uapi/linux/netfilter/nf_tables.h
+++ b/include/uapi/linux/netfilter/nf_tables.h
@@ -774,7 +774,7 @@ enum nft_reject_inet_code {
 	NFT_REJECT_ICMPX_ADMIN_PROHIBITED,
 	__NFT_REJECT_ICMPX_MAX
 };
-#define NFT_REJECT_ICMPX_MAX	(__NFT_REJECT_ICMPX_MAX + 1)
+#define NFT_REJECT_ICMPX_MAX	(__NFT_REJECT_ICMPX_MAX - 1)
 
 /**
  * enum nft_reject_attributes - nf_tables reject expression netlink attributes
diff --git a/net/netfilter/nft_reject.c b/net/netfilter/nft_reject.c
index ec8a456..57d3e1a 100644
--- a/net/netfilter/nft_reject.c
+++ b/net/netfilter/nft_reject.c
@@ -72,7 +72,7 @@ nla_put_failure:
 }
 EXPORT_SYMBOL_GPL(nft_reject_dump);
 
-static u8 icmp_code_v4[NFT_REJECT_ICMPX_MAX] = {
+static u8 icmp_code_v4[NFT_REJECT_ICMPX_MAX + 1] = {
 	[NFT_REJECT_ICMPX_NO_ROUTE]		= ICMP_NET_UNREACH,
 	[NFT_REJECT_ICMPX_PORT_UNREACH]		= ICMP_PORT_UNREACH,
 	[NFT_REJECT_ICMPX_HOST_UNREACH]		= ICMP_HOST_UNREACH,
@@ -81,8 +81,7 @@ static u8 icmp_code_v4[NFT_REJECT_ICMPX_MAX] = {
 
 int nft_reject_icmp_code(u8 code)
 {
-	if (code > NFT_REJECT_ICMPX_MAX)
-		return -EINVAL;
+	BUG_ON(code > NFT_REJECT_ICMPX_MAX);
 
 	return icmp_code_v4[code];
 }
@@ -90,7 +89,7 @@ int nft_reject_icmp_code(u8 code)
 EXPORT_SYMBOL_GPL(nft_reject_icmp_code);
 
 
-static u8 icmp_code_v6[NFT_REJECT_ICMPX_MAX] = {
+static u8 icmp_code_v6[NFT_REJECT_ICMPX_MAX + 1] = {
 	[NFT_REJECT_ICMPX_NO_ROUTE]		= ICMPV6_NOROUTE,
 	[NFT_REJECT_ICMPX_PORT_UNREACH]		= ICMPV6_PORT_UNREACH,
 	[NFT_REJECT_ICMPX_HOST_UNREACH]		= ICMPV6_ADDR_UNREACH,
@@ -99,8 +98,7 @@ static u8 icmp_code_v6[NFT_REJECT_ICMPX_MAX] = {
 
 int nft_reject_icmpv6_code(u8 code)
 {
-	if (code > NFT_REJECT_ICMPX_MAX)
-		return -EINVAL;
+	BUG_ON(code > NFT_REJECT_ICMPX_MAX);
 
 	return icmp_code_v6[code];
 }
-- 
1.7.10.4