netfilter: nft_inner: incorrect percpu area handling under softirq

Softirq can interrupt ongoing packet from process context that is
walking over the percpu area that contains inner header offsets.

Disable bh and perform three checks before restoring the percpu inner
header offsets to validate that the percpu area is valid for this
skbuff:

1) If the NFT_PKTINFO_INNER_FULL flag is set on, then this skbuff
   has already been parsed before for inner header fetching to
   register.

2) Validate that the percpu area refers to this skbuff using the
   skbuff pointer as a cookie. If there is a cookie mismatch, then
   this skbuff needs to be parsed again.

3) Finally, validate if the percpu area refers to this tunnel type.

Only after these three checks the percpu area is restored to a on-stack
copy and bh is enabled again.

After inner header fetching, the on-stack copy is stored back to the
percpu area.

Fixes: 3a07327d10a0 ("netfilter: nft_inner: support for inner tunnel header matching")
Reported-by: syzbot+84d0441b9860f0d63285@syzkaller.appspotmail.com
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

diff --git a/include/net/netfilter/nf_tables_core.h b/include/net/netfilter/nf_tables_core.h
index ff27cb2e166207cf8b58a96944d9b07a1779afe3..03b6165756fc5db03ef589e1b23f72bc48f7046d 100644 (file)
--- a/include/net/netfilter/nf_tables_core.h
+++ b/include/net/netfilter/nf_tables_core.h
@@ -161,6 +161,7 @@ enum {
 };
 
 struct nft_inner_tun_ctx {
+       unsigned long cookie;
        u16     type;
        u16     inner_tunoff;
        u16     inner_lloff;

diff --git a/net/netfilter/nft_inner.c b/net/netfilter/nft_inner.c
index 928312d01eb1d6028a2e5098a2044df0cafbefce..817ab978d24a19c32904a1861c390dc1dd99938d 100644 (file)
--- a/net/netfilter/nft_inner.c
+++ b/net/netfilter/nft_inner.c
@@ -210,35 +210,66 @@ static int nft_inner_parse(const struct nft_inner *priv,
                           struct nft_pktinfo *pkt,
                           struct nft_inner_tun_ctx *tun_ctx)
 {
-       struct nft_inner_tun_ctx ctx = {};
        u32 off = pkt->inneroff;
 
        if (priv->flags & NFT_INNER_HDRSIZE &&
-           nft_inner_parse_tunhdr(priv, pkt, &ctx, &off) < 0)
+           nft_inner_parse_tunhdr(priv, pkt, tun_ctx, &off) < 0)
                return -1;
 
        if (priv->flags & (NFT_INNER_LL | NFT_INNER_NH)) {
-               if (nft_inner_parse_l2l3(priv, pkt, &ctx, off) < 0)
+               if (nft_inner_parse_l2l3(priv, pkt, tun_ctx, off) < 0)
                        return -1;
        } else if (priv->flags & NFT_INNER_TH) {
-               ctx.inner_thoff = off;
-               ctx.flags |= NFT_PAYLOAD_CTX_INNER_TH;
+               tun_ctx->inner_thoff = off;
+               tun_ctx->flags |= NFT_PAYLOAD_CTX_INNER_TH;
        }
 
-       *tun_ctx = ctx;
        tun_ctx->type = priv->type;
+       tun_ctx->cookie = (unsigned long)pkt->skb;
        pkt->flags |= NFT_PKTINFO_INNER_FULL;
 
        return 0;
 }
 
+static bool nft_inner_restore_tun_ctx(const struct nft_pktinfo *pkt,
+                                     struct nft_inner_tun_ctx *tun_ctx)
+{
+       struct nft_inner_tun_ctx *this_cpu_tun_ctx;
+
+       local_bh_disable();
+       this_cpu_tun_ctx = this_cpu_ptr(&nft_pcpu_tun_ctx);
+       if (this_cpu_tun_ctx->cookie != (unsigned long)pkt->skb) {
+               local_bh_enable();
+               return false;
+       }
+       *tun_ctx = *this_cpu_tun_ctx;
+       local_bh_enable();
+
+       return true;
+}
+
+static void nft_inner_save_tun_ctx(const struct nft_pktinfo *pkt,
+                                  const struct nft_inner_tun_ctx *tun_ctx)
+{
+       struct nft_inner_tun_ctx *this_cpu_tun_ctx;
+
+       local_bh_disable();
+       this_cpu_tun_ctx = this_cpu_ptr(&nft_pcpu_tun_ctx);
+       if (this_cpu_tun_ctx->cookie != tun_ctx->cookie)
+               *this_cpu_tun_ctx = *tun_ctx;
+       local_bh_enable();
+}
+
 static bool nft_inner_parse_needed(const struct nft_inner *priv,
                                   const struct nft_pktinfo *pkt,
-                                  const struct nft_inner_tun_ctx *tun_ctx)
+                                  struct nft_inner_tun_ctx *tun_ctx)
 {
        if (!(pkt->flags & NFT_PKTINFO_INNER_FULL))
                return true;
 
+       if (!nft_inner_restore_tun_ctx(pkt, tun_ctx))
+               return true;
+
        if (priv->type != tun_ctx->type)
                return true;
 
@@ -248,27 +279,29 @@ static bool nft_inner_parse_needed(const struct nft_inner *priv,
 static void nft_inner_eval(const struct nft_expr *expr, struct nft_regs *regs,
                           const struct nft_pktinfo *pkt)
 {
-       struct nft_inner_tun_ctx *tun_ctx = this_cpu_ptr(&nft_pcpu_tun_ctx);
        const struct nft_inner *priv = nft_expr_priv(expr);
+       struct nft_inner_tun_ctx tun_ctx = {};
 
        if (nft_payload_inner_offset(pkt) < 0)
                goto err;
 
-       if (nft_inner_parse_needed(priv, pkt, tun_ctx) &&
-           nft_inner_parse(priv, (struct nft_pktinfo *)pkt, tun_ctx) < 0)
+       if (nft_inner_parse_needed(priv, pkt, &tun_ctx) &&
+           nft_inner_parse(priv, (struct nft_pktinfo *)pkt, &tun_ctx) < 0)
                goto err;
 
        switch (priv->expr_type) {
        case NFT_INNER_EXPR_PAYLOAD:
-               nft_payload_inner_eval((struct nft_expr *)&priv->expr, regs, pkt, tun_ctx);
+               nft_payload_inner_eval((struct nft_expr *)&priv->expr, regs, pkt, &tun_ctx);
                break;
        case NFT_INNER_EXPR_META:
-               nft_meta_inner_eval((struct nft_expr *)&priv->expr, regs, pkt, tun_ctx);
+               nft_meta_inner_eval((struct nft_expr *)&priv->expr, regs, pkt, &tun_ctx);
                break;
        default:
                WARN_ON_ONCE(1);
                goto err;
        }
+       nft_inner_save_tun_ctx(pkt, &tun_ctx);
+
        return;
 err:
        regs->verdict.code = NFT_BREAK;