* [PATCH 1/8] xfrm6: fix out-of-bounds write in xfrm6_input_addr() when secpath is full
2026-07-29 6:50 [PATCH 0/8] pull request (net): ipsec 2026-07-29 Steffen Klassert
@ 2026-07-29 6:50 ` Steffen Klassert
2026-07-29 6:50 ` [PATCH 2/8] esp: do not unref managed frag pages in esp_ssg_unref() Steffen Klassert
` (7 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Steffen Klassert @ 2026-07-29 6:50 UTC (permalink / raw)
To: David Miller, Jakub Kicinski; +Cc: Herbert Xu, Steffen Klassert, netdev
From: Xiang Mei <xmei5@asu.edu>
The depth check in xfrm6_input_addr() is off by one:
if (1 + sp->len == XFRM_MAX_DEPTH)
goto drop;
...
sp->xvec[sp->len++] = x;
xfrm_input() can leave sp->len == XFRM_MAX_DEPTH, and the transport-mode
receive path re-enters IPv6 input via xfrm_trans_reinject() with that
secpath preserved. If the inner packet carries a destination-options HAO
option or a type-2 routing header, xfrm6_input_addr() is called with
sp->len == XFRM_MAX_DEPTH; the check (1 + 6 == 6) is false, so
sp->xvec[sp->len++] writes one slot past the 6-element xvec[]. The write
stays within the sec_path allocation (invisible to KASAN); UBSAN_BOUNDS
flags it and panics under panic_on_warn.
Use "sp->len >= XFRM_MAX_DEPTH", matching xfrm_input(). This also
restores one chain level the old check rejected at sp->len == 5.
UBSAN: array-index-out-of-bounds in net/ipv6/xfrm6_input.c:309:10
index 6 is out of range for type 'xfrm_state *[6]'
Fixes: 9473e1f631de ("[XFRM] MIPv6: Fix to input RO state correctly.")
Reported-by: Weiming Shi <bestswngs@gmail.com>
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Xiang Mei <xmei5@asu.edu>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/ipv6/xfrm6_input.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv6/xfrm6_input.c b/net/ipv6/xfrm6_input.c
index 89d0443b5307..07edef258984 100644
--- a/net/ipv6/xfrm6_input.c
+++ b/net/ipv6/xfrm6_input.c
@@ -247,7 +247,7 @@ int xfrm6_input_addr(struct sk_buff *skb, xfrm_address_t *daddr,
goto drop;
}
- if (1 + sp->len == XFRM_MAX_DEPTH) {
+ if (sp->len >= XFRM_MAX_DEPTH) {
XFRM_INC_STATS(net, LINUX_MIB_XFRMINBUFFERERROR);
goto drop;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 2/8] esp: do not unref managed frag pages in esp_ssg_unref()
2026-07-29 6:50 [PATCH 0/8] pull request (net): ipsec 2026-07-29 Steffen Klassert
2026-07-29 6:50 ` [PATCH 1/8] xfrm6: fix out-of-bounds write in xfrm6_input_addr() when secpath is full Steffen Klassert
@ 2026-07-29 6:50 ` Steffen Klassert
2026-07-31 2:15 ` Jakub Kicinski
2026-07-29 6:50 ` [PATCH 3/8] xfrm: espintcp: fix UAF during close Steffen Klassert
` (6 subsequent siblings)
8 siblings, 1 reply; 12+ messages in thread
From: Steffen Klassert @ 2026-07-29 6:50 UTC (permalink / raw)
To: David Miller, Jakub Kicinski; +Cc: Herbert Xu, Steffen Klassert, netdev
From: Maher Azzouzi <maherazz04@gmail.com>
esp_ssg_unref() releases the page references held on the source
scatterlist after the AEAD operation completes. It calls
skb_page_unref() on every frag page for an out-of-place transform
(req->src != req->dst), and in the error path of esp_output_tail()
(already_unref == true) on the request's own scatterlist.
This is wrong when the skb carries managed frags
(SKBFL_MANAGED_FRAG_REFS). Managed frags are owned by a zerocopy ubuf
and the skb does not hold a per-frag page reference; io_uring SEND_ZC
with a registered buffer attaches the bvec pages this way via
io_sg_from_iter(). The rest of the stack honours this invariant:
skb_release_data() skips the per-frag unref when SKBFL_MANAGED_FRAG_REFS
is set, and skb_zcopy_managed() is the guard used at the other unref
sites.
esp_ssg_unref() is missing that guard, so for a managed-frag skb it
drops a page reference the skb never acquired. This can underflow the
page reference count and free a page that is still in use.
Guard the function with skb_zcopy_managed() so both unref paths are
skipped for managed-frag skbs, matching skb_release_data().
Fixes: cac2661c53f3 ("esp4: Avoid skb_cow_data whenever possible")
Fixes: 03e2a30f6a27 ("esp6: Avoid skb_cow_data whenever possible")
Signed-off-by: Maher Azzouzi <maherazz04@gmail.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/ipv4/esp4.c | 7 +++++++
net/ipv6/esp6.c | 7 +++++++
2 files changed, 14 insertions(+)
diff --git a/net/ipv4/esp4.c b/net/ipv4/esp4.c
index dfc81ee969ae..fa1710e27e50 100644
--- a/net/ipv4/esp4.c
+++ b/net/ipv4/esp4.c
@@ -104,6 +104,13 @@ static void esp_ssg_unref(struct xfrm_state *x, void *tmp, struct sk_buff *skb,
struct aead_request *req;
struct scatterlist *sg;
+ /* Managed frags are owned by the zerocopy ubuf; the skb holds no
+ * per-frag page reference, so we must not drop one here. Mirrors
+ * the SKBFL_MANAGED_FRAG_REFS handling in skb_release_data().
+ */
+ if (skb_zcopy_managed(skb))
+ return;
+
if (x->props.flags & XFRM_STATE_ESN)
extralen += sizeof(struct esp_output_extra);
diff --git a/net/ipv6/esp6.c b/net/ipv6/esp6.c
index 296b57926abb..7d216b9c59f0 100644
--- a/net/ipv6/esp6.c
+++ b/net/ipv6/esp6.c
@@ -121,6 +121,13 @@ static void esp_ssg_unref(struct xfrm_state *x, void *tmp, struct sk_buff *skb,
struct aead_request *req;
struct scatterlist *sg;
+ /* Managed frags are owned by the zerocopy ubuf; the skb holds no
+ * per-frag page reference, so we must not drop one here. Mirrors
+ * the SKBFL_MANAGED_FRAG_REFS handling in skb_release_data().
+ */
+ if (skb_zcopy_managed(skb))
+ return;
+
if (x->props.flags & XFRM_STATE_ESN)
extralen += sizeof(struct esp_output_extra);
--
2.43.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 2/8] esp: do not unref managed frag pages in esp_ssg_unref()
2026-07-29 6:50 ` [PATCH 2/8] esp: do not unref managed frag pages in esp_ssg_unref() Steffen Klassert
@ 2026-07-31 2:15 ` Jakub Kicinski
0 siblings, 0 replies; 12+ messages in thread
From: Jakub Kicinski @ 2026-07-31 2:15 UTC (permalink / raw)
To: steffen.klassert; +Cc: Jakub Kicinski, davem, herbert, netdev
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
--- Patch [2]: [PATCH 2/8] esp: do not unref managed frag pages in esp_ssg_unref() ---
commit 1913e3a658480a2d9a14804dfb34715f589f403d
Author: Maher Azzouzi <maherazz04@gmail.com>
esp: do not unref managed frag pages in esp_ssg_unref()
The patch adds an early return to esp_ssg_unref() in both esp4.c and esp6.c
so that no per-frag page reference is dropped when the skb carries managed
frags (SKBFL_MANAGED_FRAG_REFS), mirroring the skip_unref handling in
skb_release_data().
A couple of questions on the commit message and on the scope of the guard.
> Fixes: cac2661c53f3 ("esp4: Avoid skb_cow_data whenever possible")
> Fixes: 03e2a30f6a27 ("esp6: Avoid skb_cow_data whenever possible")
[Severity: Medium]
[Finding: main-1-2]
[Sources: opus-5]
Are these the right commits to blame? Both are dated 2017-01-17, while
SKBFL_MANAGED_FRAG_REFS and the skb_zcopy_managed() helper used by the new
guard only arrived with 753f1ca4e1e5 ("net: introduce managed frags
infrastructure", 2022-07-12, v6.1).
The esp code as written in 2017 had nothing to be wrong about here; the
described scenario only becomes possible once managed-frag skbs can reach
the xfrm output path.
Since Fixes: tags drive stable and AUTOSEL selection, this points the patch
at trees older than v6.1, where skb_zcopy_managed() does not exist at all
(build failure) and where the described scenario cannot happen. Would the
commit that made managed-frag skbs reachable from esp_output() be the better
blame target, or no Fixes: tag plus an explicit stable range?
> diff --git a/net/ipv4/esp4.c b/net/ipv4/esp4.c
> index dfc81ee969ae0..fa1710e27e505 100644
> --- a/net/ipv4/esp4.c
> +++ b/net/ipv4/esp4.c
> @@ -104,6 +104,13 @@ static void esp_ssg_unref(struct xfrm_state *x, void *tmp, struct sk_buff *skb,
> struct aead_request *req;
> struct scatterlist *sg;
>
> + /* Managed frags are owned by the zerocopy ubuf; the skb holds no
> + * per-frag page reference, so we must not drop one here. Mirrors
> + * the SKBFL_MANAGED_FRAG_REFS handling in skb_release_data().
> + */
> + if (skb_zcopy_managed(skb))
> + return;
> +
[Severity: High]
[Finding: main-1-0]
[Sources: opus-5]
Does this early return also suppress the release of the trailer page that
ESP itself referenced?
The src scatterlist looks like it has mixed ownership. Besides the
ubuf-owned payload frags, esp_output_head() adds a trailer frag with an
explicit reference in the only branch that sets esp->inplace = false:
net/ipv4/esp4.c:esp_output_head() {
...
page = pfrag->page;
get_page(page);
...
__skb_fill_page_desc(skb, nfrags, page, pfrag->offset,
tailen);
skb_shinfo(skb)->nr_frags = ++nfrags;
...
}
esp_output_tail() then drops that frag from the skb entirely:
net/ipv4/esp4.c:esp_output_tail() {
...
skb_shinfo(skb)->nr_frags = 1;
page = pfrag->page;
get_page(page);
/* replace page frags in skb with new page */
__skb_fill_page_desc(skb, 0, page, pfrag->offset, skb->data_len);
...
}
so the loop in esp_ssg_unref() appears to be the only remaining release
site for that trailer reference. skb_release_data() cannot reach the
orphaned frag, and for SKBFL_MANAGED_FRAG_REFS it takes the skip_unref
branch to free_head anyway.
Both esp_ssg_unref() call sites in esp_output_tail() are affected: the error
path
if (unlikely(err < 0)) {
esp_ssg_unref(x, tmp, skb, true);
and the success path
if (sg != dsg)
esp_ssg_unref(x, tmp, skb, false);
Note the loop only ever ran in exactly that non-inplace case
(already_unref || req->src != req->dst), which is also the case that adds
the trailer frag. So does the get_page() from esp_output_head() stay
unbalanced for a managed-frag skb?
Since a single stray reference pins the whole high-order x->xfrag
page_frag page, would every x->xfrag page used for trailers become
unfreeable, at packet rate?
Would it be better to distinguish ubuf-owned frags from the ESP-added
trailer frag, for instance by calling skb_zcopy_downgrade_managed(skb)
before ESP mutates the frag array, the way __ip_append_data() and
__ip6_append_data() do, rather than skipping the unref wholesale?
Related to that, the new comment says "the skb holds no per-frag page
reference". Is that accurate? It holds true for the ubuf-owned frags, but
not for the trailer frag ESP added with get_page().
[Severity: High]
[Finding: main-1-1]
[Sources: opus-5]
This isn't a bug introduced by this patch, but it is the invariant the
guard works around, and it stays unfixed: esp_output_tail() replaces the
frag array of a managed-frag skb with a page it references itself, while
SKBFL_MANAGED_FRAG_REFS remains set:
net/ipv4/esp4.c:esp_output_tail() {
...
skb_shinfo(skb)->nr_frags = 1;
page = pfrag->page;
get_page(page);
/* replace page frags in skb with new page */
__skb_fill_page_desc(skb, 0, page, pfrag->offset, skb->data_len);
...
}
That destination page reference is owned by frag 0, but with the flag still
set skb_release_data() takes the skip_unref branch:
net/core/skbuff.c:skb_release_data() {
...
if (skb_zcopy(skb)) {
bool skip_unref = shinfo->flags & SKBFL_MANAGED_FRAG_REFS;
skb_zcopy_clear(skb, true);
if (skip_unref)
goto free_head;
}
...
}
so is that reference ever dropped? Neither net/ipv4/esp4.c nor
net/ipv6/esp6.c calls skb_zcopy_downgrade_managed(); the canonical way to
make a managed-frag skb safe for frag mutation and per-frag unref is:
net/core/skbuff.c:__skb_zcopy_downgrade_managed() {
skb_shinfo(skb)->flags &= ~SKBFL_MANAGED_FRAG_REFS;
for (i = 0; i < skb_shinfo(skb)->nr_frags; i++)
skb_frag_ref(skb, i);
}
which every other frag-mutating site uses (net/core/skbuff.c,
net/ipv4/ip_output.c, net/ipv6/ip6_output.c, net/ipv4/tcp.c).
Would a single skb_zcopy_downgrade_managed(skb) before ESP touches the
frags address both the original refcount underflow and this leak, instead
of leaving a mixed managed/refcounted frag array as an ESP-specific special
case? In that light, is the commit message statement "The rest of the
stack honours this invariant" only half applied here?
> if (x->props.flags & XFRM_STATE_ESN)
> extralen += sizeof(struct esp_output_extra);
>
> diff --git a/net/ipv6/esp6.c b/net/ipv6/esp6.c
> index 296b57926abb9..7d216b9c59f04 100644
> --- a/net/ipv6/esp6.c
> +++ b/net/ipv6/esp6.c
> @@ -121,6 +121,13 @@ static void esp_ssg_unref(struct xfrm_state *x, void *tmp, struct sk_buff *skb,
> struct aead_request *req;
> struct scatterlist *sg;
>
> + /* Managed frags are owned by the zerocopy ubuf; the skb holds no
> + * per-frag page reference, so we must not drop one here. Mirrors
> + * the SKBFL_MANAGED_FRAG_REFS handling in skb_release_data().
> + */
> + if (skb_zcopy_managed(skb))
> + return;
> +
[Severity: High]
[Finding: main-1-0]
[Sources: opus-5]
Do both questions above apply identically to the IPv6 copy?
esp6_output_head() takes the same get_page() for the trailer frag,
esp6_output_tail() performs the same nr_frags = 1 frag replacement, and both
esp_ssg_unref() calls (already_unref == true in the skb_to_sgvec() error
path, and the sg != dsg call after encryption) return early here too.
> if (x->props.flags & XFRM_STATE_ESN)
> extralen += sizeof(struct esp_output_extra);
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 3/8] xfrm: espintcp: fix UAF during close
2026-07-29 6:50 [PATCH 0/8] pull request (net): ipsec 2026-07-29 Steffen Klassert
2026-07-29 6:50 ` [PATCH 1/8] xfrm6: fix out-of-bounds write in xfrm6_input_addr() when secpath is full Steffen Klassert
2026-07-29 6:50 ` [PATCH 2/8] esp: do not unref managed frag pages in esp_ssg_unref() Steffen Klassert
@ 2026-07-29 6:50 ` Steffen Klassert
2026-07-29 6:50 ` [PATCH 4/8] xfrm: drop ESP-in-TCP packets with no ingress device Steffen Klassert
` (5 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Steffen Klassert @ 2026-07-29 6:50 UTC (permalink / raw)
To: David Miller, Jakub Kicinski; +Cc: Herbert Xu, Steffen Klassert, netdev
From: Sabrina Dubroca <sd@queasysnail.net>
ZDI reported and analyzed a race condition during close for espintcp
sockets:
espintcp_close() frees emsg->skb via kfree_skb() without holding
any socket lock. Concurrently, the xfrm_trans_reinject work queue
invokes esp_output_tcp_finish() -> espintcp_push_skb() ->
espintcp_push_msgs() -> skb_send_sock_locked(), which reads the
same skb as a data source.
Fix this by adding a synchronize_rcu() call after resetting sk_prot,
since esp_output_tcp_finish() runs under RCU and won't use a socket
with sk_prot == &tcp_prot. Simply taking the socket lock in
espintcp_close() could lead to leaks, if esp_output_tcp_finish()
re-adds an skb in the slot we just freed. After this, the existing
barrier() is no longer needed.
Cc: stable@vger.kernel.org
Fixes: e27cca96cd68 ("xfrm: add espintcp (RFC 8229)")
Reported-by: zdi-disclosures@trendmicro.com
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
Reviewed-by: Breno Leitao <leitao@debian.org>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/xfrm/espintcp.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/xfrm/espintcp.c b/net/xfrm/espintcp.c
index 374e1b964438..cd817b855ba1 100644
--- a/net/xfrm/espintcp.c
+++ b/net/xfrm/espintcp.c
@@ -515,7 +515,8 @@ static void espintcp_close(struct sock *sk, long timeout)
strp_stop(&ctx->strp);
sk->sk_prot = &tcp_prot;
- barrier();
+
+ synchronize_rcu();
disable_work_sync(&ctx->work);
strp_done(&ctx->strp);
--
2.43.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 4/8] xfrm: drop ESP-in-TCP packets with no ingress device
2026-07-29 6:50 [PATCH 0/8] pull request (net): ipsec 2026-07-29 Steffen Klassert
` (2 preceding siblings ...)
2026-07-29 6:50 ` [PATCH 3/8] xfrm: espintcp: fix UAF during close Steffen Klassert
@ 2026-07-29 6:50 ` Steffen Klassert
2026-07-29 6:50 ` [PATCH 5/8] xfrm: avoid lock inversion in nat keepalive work Steffen Klassert
` (4 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Steffen Klassert @ 2026-07-29 6:50 UTC (permalink / raw)
To: David Miller, Jakub Kicinski; +Cc: Herbert Xu, Steffen Klassert, netdev
From: Zhiling Zou <roxy520tt@gmail.com>
ESP-in-TCP receives records through the TCP strparser. handle_esp()
restores skb->dev from the saved skb_iif before passing the packet into
the XFRM input path.
Queued TCP data can be processed after the original ingress device has
been removed, for example during veth or net namespace teardown. In that
case dev_get_by_index_rcu() returns NULL. The XFRM IPv4 and IPv6 input
paths both expect skb->dev to be valid while building the route lookup,
so queued ESP-in-TCP data can dereference a NULL device.
Drop the packet if the saved ingress device can no longer be resolved.
Such a packet can no longer be routed through the normal XFRM receive
path, and this preserves the existing behaviour for packets whose ingress
device still exists.
Fixes: e27cca96cd68 ("xfrm: add espintcp (RFC 8229)")
Cc: stable@vger.kernel.org
Reported-by: Vega <vega@nebusec.ai>
Signed-off-by: Zhiling Zou <roxy520tt@gmail.com>
Assisted-by: Codex:gpt-5.4
Reviewed-by: Ren Wei <enjou1224z@gmail.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/xfrm/espintcp.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/net/xfrm/espintcp.c b/net/xfrm/espintcp.c
index cd817b855ba1..674aedc5af5a 100644
--- a/net/xfrm/espintcp.c
+++ b/net/xfrm/espintcp.c
@@ -37,6 +37,11 @@ static void handle_esp(struct sk_buff *skb, struct sock *sk)
rcu_read_lock();
skb->dev = dev_get_by_index_rcu(sock_net(sk), skb->skb_iif);
+ if (!skb->dev) {
+ XFRM_INC_STATS(sock_net(sk), LINUX_MIB_XFRMINERROR);
+ kfree_skb(skb);
+ goto out;
+ }
local_bh_disable();
#if IS_ENABLED(CONFIG_IPV6)
if (sk->sk_family == AF_INET6)
@@ -45,6 +50,7 @@ static void handle_esp(struct sk_buff *skb, struct sock *sk)
#endif
xfrm4_rcv_encap(skb, IPPROTO_ESP, 0, TCP_ENCAP_ESPINTCP);
local_bh_enable();
+out:
rcu_read_unlock();
}
--
2.43.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 5/8] xfrm: avoid lock inversion in nat keepalive work
2026-07-29 6:50 [PATCH 0/8] pull request (net): ipsec 2026-07-29 Steffen Klassert
` (3 preceding siblings ...)
2026-07-29 6:50 ` [PATCH 4/8] xfrm: drop ESP-in-TCP packets with no ingress device Steffen Klassert
@ 2026-07-29 6:50 ` Steffen Klassert
2026-07-31 2:15 ` Jakub Kicinski
2026-07-29 6:50 ` [PATCH 6/8] xfrm: Fix skb double-free in xfrm_dev_direct_output() Steffen Klassert
` (3 subsequent siblings)
8 siblings, 1 reply; 12+ messages in thread
From: Steffen Klassert @ 2026-07-29 6:50 UTC (permalink / raw)
To: David Miller, Jakub Kicinski; +Cc: Herbert Xu, Steffen Klassert, netdev
From: Zihan Xi <xizh2024@lzu.edu.cn>
nat_keepalive_work() walks the state table while xfrm_state_walk()
holds net->xfrm.xfrm_state_lock. Its callback then acquires x->lock,
which conflicts with the delete path taking the same locks in reverse
order via xfrm_state_delete() and __xfrm_state_delete(). This creates
an AB-BA deadlock that is reported by lockdep when a NAT keepalive
worker races with SA deletion.
Fix this by splitting the keepalive walk into two phases. First,
collect the candidate states while the walk holds xfrm_state_lock and
take a reference on each state. Then, after the walk completes, process
each collected state and acquire x->lock without nesting it under
xfrm_state_lock.
Fixes: f531d13bdfe3 ("xfrm: support sending NAT keepalives in ESP in UDP states")
Cc: stable@vger.kernel.org
Reported-by: Vega <vega@nebusec.ai>
Assisted-by: Codex:gpt-5.4
Signed-off-by: Zihan Xi <xizh2024@lzu.edu.cn>
Signed-off-by: Ren Wei <enjou1224z@gmail.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/xfrm/xfrm_nat_keepalive.c | 57 +++++++++++++++++++++++++++++------
1 file changed, 48 insertions(+), 9 deletions(-)
diff --git a/net/xfrm/xfrm_nat_keepalive.c b/net/xfrm/xfrm_nat_keepalive.c
index eb1b6f67739e..8679c68c10a1 100644
--- a/net/xfrm/xfrm_nat_keepalive.c
+++ b/net/xfrm/xfrm_nat_keepalive.c
@@ -156,24 +156,51 @@ static void nat_keepalive_send(struct nat_keepalive *ka)
}
struct nat_keepalive_work_ctx {
+ struct list_head states;
time64_t next_run;
time64_t now;
};
-static int nat_keepalive_work_single(struct xfrm_state *x, int count, void *ptr)
+struct nat_keepalive_state {
+ struct list_head list;
+ struct xfrm_state *x;
+};
+
+static int nat_keepalive_work_collect(struct xfrm_state *x, int count, void *ptr)
{
struct nat_keepalive_work_ctx *ctx = ptr;
+ struct nat_keepalive_state *state;
+
+ if (!READ_ONCE(x->nat_keepalive_interval))
+ return 0;
+
+ state = kmalloc_obj(*state, GFP_ATOMIC);
+ if (!state)
+ return -ENOMEM;
+
+ xfrm_state_hold(x);
+ state->x = x;
+ list_add_tail(&state->list, &ctx->states);
+ return 0;
+}
+
+static void nat_keepalive_work_single(struct xfrm_state *x,
+ struct nat_keepalive_work_ctx *ctx)
+{
bool send_keepalive = false;
struct nat_keepalive ka;
- time64_t next_run;
+ time64_t next_run = 0;
u32 interval;
int delta;
+ spin_lock_bh(&x->lock);
+
+ if (x->km.state == XFRM_STATE_DEAD)
+ goto out;
+
interval = x->nat_keepalive_interval;
if (!interval)
- return 0;
-
- spin_lock(&x->lock);
+ goto out;
delta = (int)(ctx->now - x->lastused);
if (delta < interval) {
@@ -187,29 +214,41 @@ static int nat_keepalive_work_single(struct xfrm_state *x, int count, void *ptr)
send_keepalive = true;
}
- spin_unlock(&x->lock);
+out:
+ spin_unlock_bh(&x->lock);
if (send_keepalive)
nat_keepalive_send(&ka);
- if (!ctx->next_run || next_run < ctx->next_run)
+ if (next_run && (!ctx->next_run || next_run < ctx->next_run))
ctx->next_run = next_run;
- return 0;
}
static void nat_keepalive_work(struct work_struct *work)
{
+ struct nat_keepalive_state *state, *tmp;
struct nat_keepalive_work_ctx ctx;
struct xfrm_state_walk walk;
struct net *net;
+ int err;
+ INIT_LIST_HEAD(&ctx.states);
ctx.next_run = 0;
ctx.now = ktime_get_real_seconds();
net = container_of(work, struct net, xfrm.nat_keepalive_work.work);
xfrm_state_walk_init(&walk, IPPROTO_ESP, NULL);
- xfrm_state_walk(net, &walk, nat_keepalive_work_single, &ctx);
+ err = xfrm_state_walk(net, &walk, nat_keepalive_work_collect, &ctx);
xfrm_state_walk_done(&walk, net);
+ list_for_each_entry_safe(state, tmp, &ctx.states, list) {
+ nat_keepalive_work_single(state->x, &ctx);
+ xfrm_state_put(state->x);
+ kfree(state);
+ }
+ if (err == -ENOMEM) {
+ schedule_delayed_work(&net->xfrm.nat_keepalive_work, 0);
+ return;
+ }
if (ctx.next_run)
schedule_delayed_work(&net->xfrm.nat_keepalive_work,
(ctx.next_run - ctx.now) * HZ);
--
2.43.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 5/8] xfrm: avoid lock inversion in nat keepalive work
2026-07-29 6:50 ` [PATCH 5/8] xfrm: avoid lock inversion in nat keepalive work Steffen Klassert
@ 2026-07-31 2:15 ` Jakub Kicinski
0 siblings, 0 replies; 12+ messages in thread
From: Jakub Kicinski @ 2026-07-31 2:15 UTC (permalink / raw)
To: steffen.klassert; +Cc: Jakub Kicinski, davem, herbert, netdev
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
--- Patch [5]: [PATCH 5/8] xfrm: avoid lock inversion in nat keepalive work ---
commit 550991d4355a9993b144d945cc62ff754d14ede8
Author: Zihan Xi <xizh2024@lzu.edu.cn>
xfrm: avoid lock inversion in nat keepalive work
This splits nat_keepalive_work() into two phases: the xfrm_state_walk()
callback now only collects candidate states and takes a reference on each,
and the actual keepalive processing (which takes x->lock) happens after the
walk has dropped net->xfrm.xfrm_state_lock.
> diff --git a/net/xfrm/xfrm_nat_keepalive.c b/net/xfrm/xfrm_nat_keepalive.c
> index eb1b6f67739e1..8679c68c10a16 100644
> --- a/net/xfrm/xfrm_nat_keepalive.c
> +++ b/net/xfrm/xfrm_nat_keepalive.c
> @@ -156,24 +156,51 @@ static void nat_keepalive_send(struct nat_keepalive *ka)
> }
>
> struct nat_keepalive_work_ctx {
> + struct list_head states;
> time64_t next_run;
> time64_t now;
> };
>
> -static int nat_keepalive_work_single(struct xfrm_state *x, int count, void *ptr)
> +struct nat_keepalive_state {
> + struct list_head list;
> + struct xfrm_state *x;
> +};
> +
> +static int nat_keepalive_work_collect(struct xfrm_state *x, int count, void *ptr)
> {
> struct nat_keepalive_work_ctx *ctx = ptr;
> + struct nat_keepalive_state *state;
> +
> + if (!READ_ONCE(x->nat_keepalive_interval))
> + return 0;
> +
> + state = kmalloc_obj(*state, GFP_ATOMIC);
> + if (!state)
> + return -ENOMEM;
[Severity: Medium]
[Finding: main-1-1]
[Sources: opus-5]
Does the allocation footprint here scale with the number of SAs in a way
that is a concern? nat_keepalive_work_collect() runs as the
xfrm_state_walk() callback, so it executes under
spin_lock_bh(&net->xfrm.xfrm_state_lock), which forces GFP_ATOMIC.
One node is allocated from the __GFP_HIGH reserves, and one
xfrm_state_hold() is taken, for every ESP SA with nat_keepalive_interval
set. On a gateway with a large number of keepalive SAs that is a single
burst of tens or hundreds of thousands of GFP_ATOMIC allocations per
round, and a single failure aborts the entire round (see below).
For reference, the lock hold time itself is not a regression here: the
baseline callback already did alloc_skb(GFP_ATOMIC) and transmitted under
the same lock, so that part is if anything improved. The concerns are the
new per-SA reserve draw, the new abort-on-ENOMEM failure mode, and the
batch-wide reference pinning.
Was a non-allocating variant considered, for example READ_ONCE/WRITE_ONCE
or a dedicated lock for the three shared fields x->lastused,
x->nat_keepalive_interval and x->nat_keepalive_expiration, a persistent
struct xfrm_state_walk using the walk's stop/resume support, a per-net list
of keepalive-enabled SAs, or a bounded pre-allocated chunk?
[Severity: Low]
[Finding: main-1-2]
[Sources: opus-5]
This isn't a bug, but the patch carries:
Fixes: f531d13bdfe3 ("xfrm: support sending NAT keepalives in ESP in UDP states")
Cc: stable@vger.kernel.org
while using kmalloc_obj(), which is a recent addition in include/linux/slab.h:
#define kmalloc_obj(VAR_OR_TYPE, ...) \
__alloc_objs(kmalloc, default_gfp(__VA_ARGS__), typeof(VAR_OR_TYPE), 1)
Stable branches containing f531d13bdfe3 do not have that macro, so the
backport will not build without manual conversion. Would plain
kmalloc(sizeof(*state), GFP_ATOMIC) be preferable, since the macro buys
nothing here?
> +
> + xfrm_state_hold(x);
> + state->x = x;
> + list_add_tail(&state->list, &ctx->states);
> + return 0;
> +}
> +
> +static void nat_keepalive_work_single(struct xfrm_state *x,
> + struct nat_keepalive_work_ctx *ctx)
> +{
> bool send_keepalive = false;
> struct nat_keepalive ka;
> - time64_t next_run;
> + time64_t next_run = 0;
> u32 interval;
> int delta;
>
> + spin_lock_bh(&x->lock);
> +
> + if (x->km.state == XFRM_STATE_DEAD)
> + goto out;
> +
> interval = x->nat_keepalive_interval;
> if (!interval)
> - return 0;
> -
> - spin_lock(&x->lock);
> + goto out;
>
> delta = (int)(ctx->now - x->lastused);
> if (delta < interval) {
> @@ -187,29 +214,41 @@ static int nat_keepalive_work_single(struct xfrm_state *x, int count, void *ptr)
> send_keepalive = true;
> }
>
> - spin_unlock(&x->lock);
> +out:
> + spin_unlock_bh(&x->lock);
>
> if (send_keepalive)
> nat_keepalive_send(&ka);
[Severity: High]
[Finding: main-3-5]
[Sources: opus-5]
Is nat_keepalive_send() still called from a softirq/BH-disabled context
after this change? spin_unlock_bh() re-enables softirqs, and the phase-two
drain loop in nat_keepalive_work() has no local_bh_disable(), so this call
now runs from fully preemptible process context.
nat_keepalive_send() ends up in nat_keepalive_send_ipv4() and
nat_keepalive_send_ipv6(), both of which do:
local_lock_nested_bh(&nat_keepalive_sk_ipv4.bh_lock);
sk = this_cpu_read(nat_keepalive_sk_ipv4.sock);
sock_net_set(sk, net);
...
sock_net_set(sk, &init_net);
local_unlock_nested_bh(&nat_keepalive_sk_ipv4.bh_lock);
and local_lock_nested_bh() starts with an assertion:
#define __local_lock_nested_bh(lock) \
do { \
lockdep_assert_in_softirq(); \
local_lock_acquire((lock)); \
Before this patch nat_keepalive_work_single() was itself the
xfrm_state_walk() callback and therefore always ran with
spin_lock_bh(&net->xfrm.xfrm_state_lock) held, so in_softirq() was true.
Does every keepalive transmission now hit that WARN_ON_ONCE, replacing the
lockdep splat the changelog aims to fix with a different one?
Beyond the warning, is the exclusion for the per-CPU socket still there?
On !PREEMPT_RT __local_lock_nested_bh() is lockdep bookkeeping only, so
with BHs enabled nothing serializes the
this_cpu_read(sock)/sock_net_set(sk, net)/xmit/sock_net_set(sk, &init_net)
window. Can a preempted or migrated worker let another netns' keepalive
work clobber the same per-CPU sk, so a keepalive is emitted with the
socket's net pointing at init_net or another netns?
On PREEMPT_RT, local_lock_nested_bh() and local_unlock_nested_bh() each
resolve __this_cpu_local_lock() and take/release a real spinlock with no
migrate_disable(). Without BHs disabled, can the task migrate between
acquire and release and unlock a different CPU's lock, leaving the original
one held forever?
Would wrapping the nat_keepalive_send(&ka) call, or the whole phase-two
loop, in local_bh_disable()/local_bh_enable() be enough?
>
> - if (!ctx->next_run || next_run < ctx->next_run)
> + if (next_run && (!ctx->next_run || next_run < ctx->next_run))
> ctx->next_run = next_run;
> - return 0;
> }
>
> static void nat_keepalive_work(struct work_struct *work)
> {
> + struct nat_keepalive_state *state, *tmp;
> struct nat_keepalive_work_ctx ctx;
> struct xfrm_state_walk walk;
> struct net *net;
> + int err;
>
> + INIT_LIST_HEAD(&ctx.states);
> ctx.next_run = 0;
> ctx.now = ktime_get_real_seconds();
>
> net = container_of(work, struct net, xfrm.nat_keepalive_work.work);
> xfrm_state_walk_init(&walk, IPPROTO_ESP, NULL);
> - xfrm_state_walk(net, &walk, nat_keepalive_work_single, &ctx);
> + err = xfrm_state_walk(net, &walk, nat_keepalive_work_collect, &ctx);
> xfrm_state_walk_done(&walk, net);
> + list_for_each_entry_safe(state, tmp, &ctx.states, list) {
> + nat_keepalive_work_single(state->x, &ctx);
> + xfrm_state_put(state->x);
> + kfree(state);
> + }
> + if (err == -ENOMEM) {
> + schedule_delayed_work(&net->xfrm.nat_keepalive_work, 0);
> + return;
> + }
[Severity: High]
[Finding: main-1-0]
[Sources: opus-5]
Can this retry path make forward progress?
xfrm_state_walk() aborts the walk on any non-zero callback return and only
records a resume cursor in walk:
err = func(state, walk->seq, data);
if (err) {
list_move_tail(&walk->all, &x->all);
goto out;
}
but xfrm_state_walk_done(&walk, net) is called unconditionally right after,
and it does list_del(&walk->all), throwing that cursor away. walk is a
stack variable that xfrm_state_walk_init() re-initialises on every
invocation.
Since WORK_STRUCT_PENDING is cleared before the handler runs, the
schedule_delayed_work(..., 0) here succeeds and the work re-executes
essentially immediately, restarting from the head of net->xfrm.state_all
and failing at the same point while the atomic reserves are still
depleted. There is no backoff and no retry bound, and nothing is retired
between attempts.
Does this also duplicate keepalive packets? The send branch in
nat_keepalive_work_single():
} else {
next_run = ctx->now + interval;
nat_keepalive_init(&ka, x);
send_keepalive = true;
}
computes a local next_run but never updates x->nat_keepalive_expiration, so
on each immediate retry within the same second every SA before the failure
point still satisfies delta >= interval and
x->nat_keepalive_expiration <= ctx->now, and sends again.
And do SAs positioned after the failure point ever get serviced, given the
walk always restarts from the head and the ctx.next_run schedule computed
for the states that were processed is discarded by the early return?
Would returning 0 from nat_keepalive_work_collect() to skip the
unallocatable entry (keeping the normal ctx.next_run schedule), or resuming
the saved walk cursor with a bounded backoff, be preferable?
The changelog describes this as a locking-order fix only and does not
mention the new retry behaviour. Could it be documented there?
> if (ctx.next_run)
> schedule_delayed_work(&net->xfrm.nat_keepalive_work,
> (ctx.next_run - ctx.now) * HZ);
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 6/8] xfrm: Fix skb double-free in xfrm_dev_direct_output()
2026-07-29 6:50 [PATCH 0/8] pull request (net): ipsec 2026-07-29 Steffen Klassert
` (4 preceding siblings ...)
2026-07-29 6:50 ` [PATCH 5/8] xfrm: avoid lock inversion in nat keepalive work Steffen Klassert
@ 2026-07-29 6:50 ` Steffen Klassert
2026-07-29 6:50 ` [PATCH 7/8] xfrm: ah6: validate routing header segments_left Steffen Klassert
` (2 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Steffen Klassert @ 2026-07-29 6:50 UTC (permalink / raw)
To: David Miller, Jakub Kicinski; +Cc: Herbert Xu, Steffen Klassert, netdev
From: Sanghyun Park <sanghyun.park.cnu@gmail.com>
A return value other than 1 from local_out() means that the skb has been
consumed or its ownership was transferred. xfrm_dev_direct_output()
nevertheless frees the skb on this path, causing a double-free when
netfilter drops the packet and invalidating any other owner.
Return the local_out() result directly, matching the ownership handling
in xfrm_output_resume().
Fixes: 5eddd76ec2fd ("xfrm: fix tunnel mode TX datapath in packet offload mode")
Signed-off-by: Sanghyun Park <sanghyun.park.cnu@gmail.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/xfrm/xfrm_output.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/net/xfrm/xfrm_output.c b/net/xfrm/xfrm_output.c
index cc35c2fcbbe0..e305ba32e356 100644
--- a/net/xfrm/xfrm_output.c
+++ b/net/xfrm/xfrm_output.c
@@ -636,10 +636,8 @@ static int xfrm_dev_direct_output(struct sock *sk, struct xfrm_state *x,
nf_reset_ct(skb);
err = skb_dst(skb)->ops->local_out(net, sk, skb);
- if (unlikely(err != 1)) {
- kfree_skb(skb);
+ if (unlikely(err != 1))
return err;
- }
/* In transport mode, network destination is
* directly reachable, while in tunnel mode,
--
2.43.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 7/8] xfrm: ah6: validate routing header segments_left
2026-07-29 6:50 [PATCH 0/8] pull request (net): ipsec 2026-07-29 Steffen Klassert
` (5 preceding siblings ...)
2026-07-29 6:50 ` [PATCH 6/8] xfrm: Fix skb double-free in xfrm_dev_direct_output() Steffen Klassert
@ 2026-07-29 6:50 ` Steffen Klassert
2026-07-29 6:50 ` [PATCH 8/8] xfrm: fix xfrm_state_construct() auth-trunc leak Steffen Klassert
2026-07-31 2:17 ` [PATCH 0/8] pull request (net): ipsec 2026-07-29 Jakub Kicinski
8 siblings, 0 replies; 12+ messages in thread
From: Steffen Klassert @ 2026-07-29 6:50 UTC (permalink / raw)
To: David Miller, Jakub Kicinski; +Cc: Herbert Xu, Steffen Klassert, netdev
From: Asim Viladi Oglu Manizada <manizada@pm.me>
AH6 rearranges routing-header addresses before computing or verifying the
ICV. ipv6_rearrange_rthdr() assumes that segments_left is not larger than
the number of addresses described by the routing header's hdrlen field.
That assumption does not hold for raw IPv6 HDRINCL packets. A packet with
hdrlen equal to 2 describes one address, but can carry an arbitrary
segments_left value. With segments_left equal to 255, the function moves
its address pointer 4,064 bytes backwards and passes a 4,064-byte length to
memmove(), resulting in an out-of-bounds access.
Validate the invariant locally before modifying the routing header or
performing any address-pointer arithmetic, and propagate malformed-header
errors to the existing AH6 input and output error paths.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Assisted-by: avom-custom-harness:gpt-5.5-qwen3.6-mod-mix
Signed-off-by: Asim Viladi Oglu Manizada <manizada@pm.me>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/ipv6/ah6.c | 29 ++++++++++++++++++-----------
1 file changed, 18 insertions(+), 11 deletions(-)
diff --git a/net/ipv6/ah6.c b/net/ipv6/ah6.c
index 76f7a2de9108..c96f7e0d0a48 100644
--- a/net/ipv6/ah6.c
+++ b/net/ipv6/ah6.c
@@ -232,26 +232,28 @@ static void ipv6_rearrange_destopt(struct ipv6hdr *iph, struct ipv6_opt_hdr *des
* Rearrange the destination address in @iph and the addresses in @rthdr
* so that they appear in the order they will at the final destination.
* See Appendix A2 of RFC 2402 for details.
+ *
+ * Return: 0 on success, -EINVAL if segments_left exceeds the number of
+ * addresses described by hdrlen.
*/
-static void ipv6_rearrange_rthdr(struct ipv6hdr *iph, struct ipv6_rt_hdr *rthdr)
+static int ipv6_rearrange_rthdr(struct ipv6hdr *iph, struct ipv6_rt_hdr *rthdr)
{
- int segments, segments_left;
+ unsigned int segments, segments_left;
struct in6_addr *addrs;
struct in6_addr final_addr;
segments_left = rthdr->segments_left;
if (segments_left == 0)
- return;
- rthdr->segments_left = 0;
+ return 0;
- /* The value of rthdr->hdrlen has been verified either by the system
- * call if it is locally generated, or by ipv6_rthdr_rcv() for incoming
- * packets. So we can assume that it is even and that segments is
- * greater than or equal to segments_left.
- *
- * For the same reason we can assume that this option is of type 0.
+ /* Raw locally generated packets can reach AH6 without the invariant
+ * required by the rt0-style address rearrangement below.
*/
segments = rthdr->hdrlen >> 1;
+ if (segments_left > segments)
+ return -EINVAL;
+
+ rthdr->segments_left = 0;
addrs = ((struct rt0_hdr *)rthdr)->addr;
final_addr = addrs[segments - 1];
@@ -261,6 +263,8 @@ static void ipv6_rearrange_rthdr(struct ipv6hdr *iph, struct ipv6_rt_hdr *rthdr)
addrs[0] = iph->daddr;
iph->daddr = final_addr;
+
+ return 0;
}
static int ipv6_clear_mutable_options(struct ipv6hdr *iph, int len, int dir)
@@ -273,6 +277,7 @@ static int ipv6_clear_mutable_options(struct ipv6hdr *iph, int len, int dir)
} exthdr = { .iph = iph };
char *end = exthdr.raw + len;
int nexthdr = iph->nexthdr;
+ int err;
exthdr.iph++;
@@ -292,7 +297,9 @@ static int ipv6_clear_mutable_options(struct ipv6hdr *iph, int len, int dir)
break;
case NEXTHDR_ROUTING:
- ipv6_rearrange_rthdr(iph, exthdr.rth);
+ err = ipv6_rearrange_rthdr(iph, exthdr.rth);
+ if (err)
+ return err;
break;
default:
--
2.43.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 8/8] xfrm: fix xfrm_state_construct() auth-trunc leak
2026-07-29 6:50 [PATCH 0/8] pull request (net): ipsec 2026-07-29 Steffen Klassert
` (6 preceding siblings ...)
2026-07-29 6:50 ` [PATCH 7/8] xfrm: ah6: validate routing header segments_left Steffen Klassert
@ 2026-07-29 6:50 ` Steffen Klassert
2026-07-31 2:17 ` [PATCH 0/8] pull request (net): ipsec 2026-07-29 Jakub Kicinski
8 siblings, 0 replies; 12+ messages in thread
From: Steffen Klassert @ 2026-07-29 6:50 UTC (permalink / raw)
To: David Miller, Jakub Kicinski; +Cc: Herbert Xu, Steffen Klassert, netdev
From: Zihan Xi <zihanx@nebusec.ai>
attach_auth_trunc() can allocate x->aalg while leaving
x->props.aalgo at zero when the selected auth algorithm has no
sadb_alg_id. One real case is cmac(aes).
xfrm_state_construct() then treats !x->props.aalgo as "no auth
algorithm attached yet" and calls attach_auth(). That overwrites
x->aalg and loses the first allocation. Any later failure or teardown
only frees the replacement pointer.
Check whether x->aalg is already attached instead of inferring that
state from x->props.aalgo.
Fixes: 4447bb33f094 ("xfrm: Store aalg in xfrm_state with a user specified truncation length")
Cc: stable@vger.kernel.org
Reported-by: Vega <vega@nebusec.ai>
Assisted-by: Codex:gpt-5.4
Signed-off-by: Zihan Xi <zihanx@nebusec.ai>
Signed-off-by: Ren Wei <enjou1224z@gmail.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/xfrm/xfrm_user.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index d6db63304ba6..6266a92cf302 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -940,7 +940,7 @@ static struct xfrm_state *xfrm_state_construct(struct net *net,
if ((err = attach_auth_trunc(&x->aalg, &x->props.aalgo,
attrs[XFRMA_ALG_AUTH_TRUNC], extack)))
goto error;
- if (!x->props.aalgo) {
+ if (!x->aalg) {
if ((err = attach_auth(&x->aalg, &x->props.aalgo,
attrs[XFRMA_ALG_AUTH], extack)))
goto error;
--
2.43.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 0/8] pull request (net): ipsec 2026-07-29
2026-07-29 6:50 [PATCH 0/8] pull request (net): ipsec 2026-07-29 Steffen Klassert
` (7 preceding siblings ...)
2026-07-29 6:50 ` [PATCH 8/8] xfrm: fix xfrm_state_construct() auth-trunc leak Steffen Klassert
@ 2026-07-31 2:17 ` Jakub Kicinski
8 siblings, 0 replies; 12+ messages in thread
From: Jakub Kicinski @ 2026-07-31 2:17 UTC (permalink / raw)
To: Steffen Klassert; +Cc: David Miller, Herbert Xu, netdev
On Wed, 29 Jul 2026 08:50:10 +0200 Steffen Klassert wrote:
> Please pull or let me know if there are problems.
Sorry for the delay, patch overload and Sashiko found some issues.
I haven't had time today to investigate either, so I just sent them
out as is - LMK if they are not real but AFAICT both models said
the same thing here.
^ permalink raw reply [flat|nested] 12+ messages in thread