netfilter: nf_tables: check tprot_set first when we use xt.thoff
authorLiping Zhang <liping.zhang@spreadtrum.com>
Sat, 17 Sep 2016 06:31:20 +0000 (14:31 +0800)
committerPablo Neira Ayuso <pablo@netfilter.org>
Fri, 23 Sep 2016 07:30:26 +0000 (09:30 +0200)
pkt->xt.thoff is not always set properly, but we use it without any check.
For payload expr, it will cause wrong results. For nftrace, we may notify
the wrong network or transport header to the user space, furthermore,
input the following nft rules, warning message will be printed out:
  # nft add rule arp filter output meta nftrace set 1

  WARNING: CPU: 0 PID: 13428 at net/netfilter/nf_tables_trace.c:263
  nft_trace_notify+0x4a3/0x5e0 [nf_tables]
  Call Trace:
  [<ffffffff813d58ae>] dump_stack+0x63/0x85
  [<ffffffff810a4c0b>] __warn+0xcb/0xf0
  [<ffffffff810a4d3d>] warn_slowpath_null+0x1d/0x20
  [<ffffffffa0589703>] nft_trace_notify+0x4a3/0x5e0 [nf_tables]
  [ ... ]
  [<ffffffffa05690a8>] nft_do_chain_arp+0x78/0x90 [nf_tables_arp]
  [<ffffffff816f4aa2>] nf_iterate+0x62/0x80
  [<ffffffff816f4b33>] nf_hook_slow+0x73/0xd0
  [<ffffffff81732bbf>] arp_xmit+0x8f/0xb0
  [ ... ]
  [<ffffffff81732d36>] arp_solicit+0x106/0x2c0

So before we use pkt->xt.thoff, check the tprot_set first.

Signed-off-by: Liping Zhang <liping.zhang@spreadtrum.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
net/netfilter/nf_tables_core.c
net/netfilter/nf_tables_trace.c
net/netfilter/nft_payload.c

index 36ba4e55d84e7039caa0e404ef82835b96057270..67259cefef06a391653c5f69ce3a5cfcd9618b84 100644 (file)
@@ -93,8 +93,11 @@ static bool nft_payload_fast_eval(const struct nft_expr *expr,
 
        if (priv->base == NFT_PAYLOAD_NETWORK_HEADER)
                ptr = skb_network_header(skb);
-       else
+       else {
+               if (!pkt->tprot_set)
+                       return false;
                ptr = skb_network_header(skb) + pkt->xt.thoff;
+       }
 
        ptr += priv->offset;
 
index 39eb1cc62e91fdda9608f96eb678f061f7b91a0c..696fe8f6f2f26c2dd44132fb28245a3860aad032 100644 (file)
@@ -113,20 +113,22 @@ static int nf_trace_fill_pkt_info(struct sk_buff *nlskb,
                                  const struct nft_pktinfo *pkt)
 {
        const struct sk_buff *skb = pkt->skb;
-       unsigned int len = min_t(unsigned int,
-                                pkt->xt.thoff - skb_network_offset(skb),
-                                NFT_TRACETYPE_NETWORK_HSIZE);
        int off = skb_network_offset(skb);
+       unsigned int len, nh_end;
 
+       nh_end = pkt->tprot_set ? pkt->xt.thoff : skb->len;
+       len = min_t(unsigned int, nh_end - skb_network_offset(skb),
+                   NFT_TRACETYPE_NETWORK_HSIZE);
        if (trace_fill_header(nlskb, NFTA_TRACE_NETWORK_HEADER, skb, off, len))
                return -1;
 
-       len = min_t(unsigned int, skb->len - pkt->xt.thoff,
-                   NFT_TRACETYPE_TRANSPORT_HSIZE);
-
-       if (trace_fill_header(nlskb, NFTA_TRACE_TRANSPORT_HEADER, skb,
-                             pkt->xt.thoff, len))
-               return -1;
+       if (pkt->tprot_set) {
+               len = min_t(unsigned int, skb->len - pkt->xt.thoff,
+                           NFT_TRACETYPE_TRANSPORT_HSIZE);
+               if (trace_fill_header(nlskb, NFTA_TRACE_TRANSPORT_HEADER, skb,
+                                     pkt->xt.thoff, len))
+                       return -1;
+       }
 
        if (!skb_mac_header_was_set(skb))
                return 0;
index 12cd4bf16d17d36f1c5c250771792d853443c853..b2f88617611aac450631e606174f5bfd58cd38fa 100644 (file)
@@ -92,6 +92,8 @@ static void nft_payload_eval(const struct nft_expr *expr,
                offset = skb_network_offset(skb);
                break;
        case NFT_PAYLOAD_TRANSPORT_HEADER:
+               if (!pkt->tprot_set)
+                       goto err;
                offset = pkt->xt.thoff;
                break;
        default:
@@ -184,6 +186,8 @@ static void nft_payload_set_eval(const struct nft_expr *expr,
                offset = skb_network_offset(skb);
                break;
        case NFT_PAYLOAD_TRANSPORT_HEADER:
+               if (!pkt->tprot_set)
+                       goto err;
                offset = pkt->xt.thoff;
                break;
        default: