* [PATCH net v7 1/3] xfrm: use skb->dev before dropping the reference held across async
2026-10-27 2:08 [PATCH net v7 0/3] xfrm: fix vti tunnel device use-after-free across async crypto resumption Qihang
@ 2026-10-14 13:17 ` Qihang
2026-10-14 13:17 ` [PATCH net v7 2/3] xfrm: set the address family before the early invalid-state drop Qihang
2026-10-18 7:03 ` [PATCH net v7 3/3] xfrm: hold the tunnel device reference across the input path Qihang
2 siblings, 0 replies; 4+ messages in thread
From: Qihang @ 2026-10-14 13:17 UTC (permalink / raw)
To: netdev
Cc: steffen.klassert, chopps, herbert, davem, edumazet, kuba, pabeni,
horms, dsahern, idosch, linux-kernel
The skb->dev reference that is held across an async resumption is
released before skb->dev is used, on two exits of xfrm_input():
- the drop path runs the receive callbacks after the reference is
dropped; xfrmi_rcv_cb() reads skb->dev->ifindex, so it can race a
concurrent teardown of the device
- the decaps success exit calls gro_cells_receive() after the
reference is dropped; gro_cells_receive() reads skb->dev
Use skb->dev first and drop the reference afterwards in both cases,
matching what the transport exit already does.
The early drop for a state that is no longer valid releases the
reference on its own; let the drop label handle that reference too.
Fixes: 1c428b038400 ("xfrm: hold dev ref until after transport_finish NF_HOOK")
Cc: stable@vger.kernel.org
Signed-off-by: Qihang <q.h.hack.winter@gmail.com>
---
v7:
- also move the reference drop below gro_cells_receive() in the decaps
success exit, which used skb->dev after the reference was dropped
(found by the netdev AI review).
net/xfrm/xfrm_input.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/net/xfrm/xfrm_input.c b/net/xfrm/xfrm_input.c
index 5ed87d51392a5..cfd5cea033127 100644
--- a/net/xfrm/xfrm_input.c
+++ b/net/xfrm/xfrm_input.c
@@ -496,7 +496,7 @@ int xfrm_input(struct sk_buff *skb, int nexthdr, __be32 spi, int encap_type)
LINUX_MIB_XFRMINSTATEINVALID);
if (encap_type == -1)
- dev_put(dev);
+ async = 1;
goto drop;
}
@@ -747,9 +747,10 @@ int xfrm_input(struct sk_buff *skb, int nexthdr, __be32 spi, int encap_type)
sp->olen = 0;
if (skb_valid_dst(skb))
skb_dst_drop(skb);
+ /* gro_cells_receive() reads skb->dev. */
+ gro_cells_receive(&gro_cells, skb);
if (async)
dev_put(dev);
- gro_cells_receive(&gro_cells, skb);
rcu_read_unlock();
return 0;
} else {
@@ -779,9 +780,9 @@ int xfrm_input(struct sk_buff *skb, int nexthdr, __be32 spi, int encap_type)
drop_unlock:
spin_unlock(&x->lock);
drop:
+ xfrm_rcv_cb(skb, family, xfrm_proto, -1);
if (async)
dev_put(dev);
- xfrm_rcv_cb(skb, family, xfrm_proto, -1);
kfree_skb(skb);
return 0;
}
--
2.54.0 (Apple Git-157)
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH net v7 3/3] xfrm: hold the tunnel device reference across the input path
2026-10-27 2:08 [PATCH net v7 0/3] xfrm: fix vti tunnel device use-after-free across async crypto resumption Qihang
2026-10-14 13:17 ` [PATCH net v7 1/3] xfrm: use skb->dev before dropping the reference held across async Qihang
2026-10-14 13:17 ` [PATCH net v7 2/3] xfrm: set the address family before the early invalid-state drop Qihang
@ 2026-10-18 7:03 ` Qihang
2 siblings, 0 replies; 4+ messages in thread
From: Qihang @ 2026-10-18 7:03 UTC (permalink / raw)
To: netdev
Cc: steffen.klassert, chopps, herbert, davem, edumazet, kuba, pabeni,
horms, dsahern, idosch, linux-kernel
vti_input() and vti6_input_proto() cache the tunnel pointer in the skb
control buffer without taking a reference. With async crypto the
receive callback runs from the crypto completion, after a possible
device teardown, and dereferences the freed tunnel device:
BUG: KASAN: slab-use-after-free in vti_rcv_cb
Take a reference on the tunnel device when the pointer is cached. The
tunnel is the device's private area, so the reference keeps the tunnel
structure alive as well. xfrm6_rcv_spi() takes the same reference for
the tunnel it is given; it is a separate entry point, so vti6 does not
hold the reference twice.
A successful vti/vti6 callback leaves the cached tunnel in place for
xfrm_input() to release after delivery. A callback that fails only
updates the error counters; the common drop path releases the reference
after the skb has been freed. The helper takes the device out of the
skb before gro_cells_receive() and transport_finish(), which may already
consume the skb. The device reference is therefore always dropped after
the skb is gone.
IPTFS consumes the outer skb itself and frees it directly, so it must
release the reference as well. The IPTFS state carries the address
family the tunnel was resolved for, and the caller passes it in. Add
xfrm_input_drop() and xfrm_input_consume() for the outer-skb free paths
in __input_process_payload(), iptfs_input_ordered(), iptfs_input(),
iptfs_drop_timer() and iptfs_destroy_state(). Only the outer skb
carries the cached tunnel; the inner skbs built by iptfs_alloc_skb()
have a zeroed control buffer, so they do not release it. The drop paths
keep their skb drop reason and the consumed skb keeps consume_skb()
semantics.
An inner packet that fails validation reaches the drop label before the
loop that delivers the queued inner packets runs, so the queued skbs
have to be dropped as well. That part also fixes a pre-existing leak:
those skbs, and the sec_path reference they hold on the xfrm_state, were
otherwise never freed.
The reference is released right after gro_cells_receive(); the NAPI
that processes the queued skb later still uses skb->dev without a
reference. That window is pre-existing and is not changed here.
This depends on the previous two patches: the receive callbacks must
run before the async dev_put(), and the early drop must know the
address family to release the reference.
Fixes: df3893c176e9 ("vti: Update the ipv4 side to use it's own receive hook.")
Fixes: fa9ad96d4905 ("vti6: Update the ipv6 side to use its own receive hook.")
Fixes: 6c82d24336718 ("xfrm: iptfs: add basic receive packet (tunnel egress) handling")
Cc: stable@vger.kernel.org
Signed-off-by: Qihang <q.h.hack.winter@gmail.com>
---
v7:
- add a Fixes: tag for the pre-existing IPTFS leak that the queued-skb
drop also fixes, and mention it in the description.
- rebase on the patch 1 change (decaps exit ordering).
net/ipv4/ip_vti.c | 3 ++
net/ipv6/ip6_vti.c | 2 ++
net/ipv6/xfrm6_input.c | 3 ++
net/xfrm/xfrm_inout.h | 4 +++
net/xfrm/xfrm_input.c | 64 ++++++++++++++++++++++++++++++++++++++++--
net/xfrm/xfrm_iptfs.c | 42 ++++++++++++++++++---------
6 files changed, 102 insertions(+), 16 deletions(-)
diff --git a/net/ipv4/ip_vti.c b/net/ipv4/ip_vti.c
index 3b80929994a00..0277812852e7c 100644
--- a/net/ipv4/ip_vti.c
+++ b/net/ipv4/ip_vti.c
@@ -63,6 +63,9 @@ static int vti_input(struct sk_buff *skb, int nexthdr, __be32 spi,
XFRM_TUNNEL_SKB_CB(skb)->tunnel.ip4 = tunnel;
+ /* Keep the tunnel alive until xfrm_input() is done with the skb. */
+ dev_hold(tunnel->dev);
+
if (update_skb_dev)
skb->dev = tunnel->dev;
diff --git a/net/ipv6/ip6_vti.c b/net/ipv6/ip6_vti.c
index ab94b3a4ba9c3..6715bf10bc85d 100644
--- a/net/ipv6/ip6_vti.c
+++ b/net/ipv6/ip6_vti.c
@@ -319,6 +319,8 @@ static int vti6_input_proto(struct sk_buff *skb, int nexthdr, __be32 spi,
goto discard;
}
+ /* Keep the tunnel alive until xfrm_input() is done with the skb. */
+ dev_hold(t->dev);
rcu_read_unlock();
XFRM_TUNNEL_SKB_CB(skb)->tunnel.ip6 = t;
diff --git a/net/ipv6/xfrm6_input.c b/net/ipv6/xfrm6_input.c
index 07edef2589844..d27bc24f1b160 100644
--- a/net/ipv6/xfrm6_input.c
+++ b/net/ipv6/xfrm6_input.c
@@ -15,6 +15,7 @@
#include <linux/netfilter.h>
#include <linux/netfilter_ipv6.h>
#include <net/ipv6.h>
+#include <net/ip6_tunnel.h>
#include <net/xfrm.h>
#include <net/protocol.h>
#include <net/gro.h>
@@ -23,6 +24,8 @@ int xfrm6_rcv_spi(struct sk_buff *skb, int nexthdr, __be32 spi,
struct ip6_tnl *t)
{
XFRM_TUNNEL_SKB_CB(skb)->tunnel.ip6 = t;
+ if (t)
+ dev_hold(t->dev);
XFRM_SPI_SKB_CB(skb)->family = AF_INET6;
XFRM_SPI_SKB_CB(skb)->daddroff = offsetof(struct ipv6hdr, daddr);
return xfrm_input(skb, nexthdr, spi, 0);
diff --git a/net/xfrm/xfrm_inout.h b/net/xfrm/xfrm_inout.h
index efc5e6b2e87be..8bd05f62ad562 100644
--- a/net/xfrm/xfrm_inout.h
+++ b/net/xfrm/xfrm_inout.h
@@ -6,6 +6,10 @@
#ifndef XFRM_INOUT_H
#define XFRM_INOUT_H 1
+void xfrm_input_drop(struct sk_buff *skb, unsigned int family,
+ enum skb_drop_reason reason);
+void xfrm_input_consume(struct sk_buff *skb, unsigned int family);
+
static inline void xfrm4_extract_header(struct sk_buff *skb)
{
const struct iphdr *iph = ip_hdr(skb);
diff --git a/net/xfrm/xfrm_input.c b/net/xfrm/xfrm_input.c
index ab391258a5028..2797903d078dd 100644
--- a/net/xfrm/xfrm_input.c
+++ b/net/xfrm/xfrm_input.c
@@ -120,6 +120,56 @@ static int xfrm_rcv_cb(struct sk_buff *skb, unsigned int family, u8 protocol,
return ret;
}
+/*
+ * The cached tunnel holds one reference on its net_device.
+ * The address family determines the tunnel type.
+ */
+static struct net_device *xfrm_input_take_tunnel_dev(struct sk_buff *skb,
+ unsigned int family)
+{
+ void *slot = (void *)XFRM_TUNNEL_SKB_CB(skb)->tunnel.ip4;
+ struct net_device *dev;
+
+ if (!slot)
+ return NULL;
+
+ switch (family) {
+ case AF_INET:
+ dev = ((struct ip_tunnel *)slot)->dev;
+ break;
+ case AF_INET6:
+ dev = ((struct ip6_tnl *)slot)->dev;
+ break;
+ default:
+ WARN_ON_ONCE(1);
+ return NULL;
+ }
+
+ XFRM_TUNNEL_SKB_CB(skb)->tunnel.ip4 = NULL;
+ return dev;
+}
+
+void xfrm_input_drop(struct sk_buff *skb, unsigned int family,
+ enum skb_drop_reason reason)
+{
+ struct net_device *dev = xfrm_input_take_tunnel_dev(skb, family);
+
+ kfree_skb_reason(skb, reason);
+ if (dev)
+ dev_put(dev);
+}
+EXPORT_SYMBOL(xfrm_input_drop);
+
+void xfrm_input_consume(struct sk_buff *skb, unsigned int family)
+{
+ struct net_device *dev = xfrm_input_take_tunnel_dev(skb, family);
+
+ consume_skb(skb);
+ if (dev)
+ dev_put(dev);
+}
+EXPORT_SYMBOL(xfrm_input_consume);
+
struct sec_path *secpath_set(struct sk_buff *skb)
{
struct sec_path *sp, *tmp = skb_ext_find(skb, SKB_EXT_SEC_PATH);
@@ -468,6 +518,7 @@ int xfrm_input(struct sk_buff *skb, int nexthdr, __be32 spi, int encap_type)
const struct xfrm_state_afinfo *afinfo;
struct net *net = dev_net(skb->dev);
struct net_device *dev = skb->dev;
+ struct net_device *tdev;
int err;
__be32 seq;
__be32 seq_hi;
@@ -746,8 +797,11 @@ int xfrm_input(struct sk_buff *skb, int nexthdr, __be32 spi, int encap_type)
sp->olen = 0;
if (skb_valid_dst(skb))
skb_dst_drop(skb);
- /* gro_cells_receive() reads skb->dev. */
+ /* gro_cells_receive() may free the skb. */
+ tdev = xfrm_input_take_tunnel_dev(skb, family);
gro_cells_receive(&gro_cells, skb);
+ if (tdev)
+ dev_put(tdev);
if (async)
dev_put(dev);
rcu_read_unlock();
@@ -757,6 +811,7 @@ int xfrm_input(struct sk_buff *skb, int nexthdr, __be32 spi, int encap_type)
if (xo)
xfrm_gro = xo->flags & XFRM_GRO;
+ tdev = xfrm_input_take_tunnel_dev(skb, family);
err = -EAFNOSUPPORT;
afinfo = xfrm_state_afinfo_get_rcu(family);
if (likely(afinfo))
@@ -770,6 +825,8 @@ int xfrm_input(struct sk_buff *skb, int nexthdr, __be32 spi, int encap_type)
gro_cells_receive(&gro_cells, skb);
}
+ if (tdev)
+ dev_put(tdev);
if (async)
dev_put(dev);
rcu_read_unlock();
@@ -780,9 +837,12 @@ int xfrm_input(struct sk_buff *skb, int nexthdr, __be32 spi, int encap_type)
spin_unlock(&x->lock);
drop:
xfrm_rcv_cb(skb, family, xfrm_proto, -1);
+ tdev = xfrm_input_take_tunnel_dev(skb, family);
+ kfree_skb(skb);
+ if (tdev)
+ dev_put(tdev);
if (async)
dev_put(dev);
- kfree_skb(skb);
return 0;
}
EXPORT_SYMBOL(xfrm_input);
diff --git a/net/xfrm/xfrm_iptfs.c b/net/xfrm/xfrm_iptfs.c
index 6920940a35b49..2b202c7570f18 100644
--- a/net/xfrm/xfrm_iptfs.c
+++ b/net/xfrm/xfrm_iptfs.c
@@ -709,8 +709,9 @@ static void __iptfs_reassem_done(struct xfrm_iptfs_data *xtfs, bool free)
/* We don't care if it works locking takes care of things */
hrtimer_try_to_cancel(&xtfs->drop_timer);
- if (free)
- kfree_skb(xtfs->ra_newskb);
+ if (free && xtfs->ra_newskb)
+ xfrm_input_drop(xtfs->ra_newskb, xtfs->x->props.family,
+ SKB_DROP_REASON_NOT_SPECIFIED);
xtfs->ra_newskb = NULL;
}
@@ -1204,15 +1205,23 @@ static bool __input_process_payload(struct xfrm_state *x, u32 data,
skb_abort_seq_read(skbseq);
if (defer) {
- consume_skb(defer);
+ xfrm_input_consume(defer, x->props.family);
} else if (!first_skb) {
- /* skb is the original passed in skb, but we didn't get far
- * enough to process it as the first_skb, if we had it would
- * either be save in ra_newskb, trimmed and sent on as an skb or
- * placed in defer to be freed.
- */
- kfree_skb(skb);
+ /* The original skb was not consumed as the first inner packet. */
+ xfrm_input_drop(skb, x->props.family,
+ SKB_DROP_REASON_NOT_SPECIFIED);
+ }
+
+ /*
+ * A validation failure can bypass delivery of @sublist. Drop the
+ * queued skbs so the outer skb's tunnel reference is released.
+ */
+ list_for_each_entry_safe(skb, next, sublist, list) {
+ skb_list_del_init(skb);
+ xfrm_input_drop(skb, x->props.family,
+ SKB_DROP_REASON_NOT_SPECIFIED);
}
+
return true;
}
@@ -1298,7 +1307,8 @@ static void iptfs_input_ordered(struct xfrm_state *x, struct sk_buff *skb)
if (!consumed) {
skb = skbseq.root_skb;
skb_abort_seq_read(&skbseq);
- kfree_skb(skb);
+ xfrm_input_drop(skb, x->props.family,
+ SKB_DROP_REASON_NOT_SPECIFIED);
}
}
@@ -1700,7 +1710,8 @@ static enum hrtimer_restart iptfs_drop_timer(struct hrtimer *me)
spin_unlock(&xtfs->drop_lock);
if (skb)
- kfree_skb_reason(skb, SKB_DROP_REASON_FRAG_REASM_TIMEOUT);
+ xfrm_input_drop(skb, x->props.family,
+ SKB_DROP_REASON_FRAG_REASM_TIMEOUT);
if (count) {
list_for_each_entry_safe(skb, next, &list, list) {
@@ -1751,7 +1762,8 @@ static int iptfs_input(struct xfrm_state *x, struct sk_buff *skb)
list_for_each_entry_safe(skb, next, &freelist, list) {
skb_list_del_init(skb);
- kfree_skb(skb);
+ xfrm_input_drop(skb, x->props.family,
+ SKB_DROP_REASON_NOT_SPECIFIED);
}
done:
/* We always have dealt with the input SKB, either we are re-using it,
@@ -2752,11 +2764,13 @@ static void iptfs_destroy_state(struct xfrm_state *x)
hrtimer_cancel(&xtfs->drop_timer);
if (xtfs->ra_newskb)
- kfree_skb(xtfs->ra_newskb);
+ xfrm_input_drop(xtfs->ra_newskb, xtfs->x->props.family,
+ SKB_DROP_REASON_NOT_SPECIFIED);
for (s = xtfs->w_saved, se = s + xtfs->w_savedlen; s < se; s++) {
if (s->skb)
- kfree_skb(s->skb);
+ xfrm_input_drop(s->skb, x->props.family,
+ SKB_DROP_REASON_NOT_SPECIFIED);
}
kfree_sensitive(xtfs->w_saved);
--
2.54.0 (Apple Git-157)
^ permalink raw reply [flat|nested] 4+ messages in thread