Re: [PATCH 04/12] xfrm: iptfs: fix runt reassembly panic from short inner tot_len
From: Steffen Klassert <steffen.klassert@secunet.com>
Date: 2026-09-14 09:19:31
Ccing Author of the patch and author of IPTFS. On Tue, Sep 08, 2026 at 03:48:55PM -0700, Jakub Kicinski wrote:
This is an AI-generated review of your patch. The human sending this email has considered the AI review valid, or at least plausible. --- xfrm: iptfs: fix runt reassembly panic from short inner tot_len This tightens the runt continuation path in iptfs_reassem_cont() so the declared inner IP length must cover at least the IP header size before the in-progress reassembly skb is allocated, replacing the old check against sizeof(xtfs->ra_runt). A few questions below on how completely this matches the in-place validation, plus two pre-existing observations in the same function.quoted
diff --git a/net/xfrm/xfrm_iptfs.c b/net/xfrm/xfrm_iptfs.c index 2ce15c472cc4d..6920940a35b49 100644 --- a/net/xfrm/xfrm_iptfs.c +++ b/net/xfrm/xfrm_iptfs.c@@ -828,8 +828,8 @@ static u32 iptfs_reassem_cont(struct xfrm_iptfs_data *xtfs, u64 seq, * allocate an in progress skb */ ipremain = __iptfs_iplen(xtfs->ra_runt); - if (ipremain < sizeof(xtfs->ra_runt)) { - /* length has to be at least runtsize large */ + if (ipremain < __iptfs_iphlen(xtfs->ra_runt)) { + /* length has to be at least the IP header size */[Severity: Medium] The changelog says this will "Align the runt path with the normal path by requiring the declared inner length to cover at least the IP header size." Is that alignment complete for IPv4 with options? __iptfs_iphlen() never looks at iph->ihl: net/xfrm/xfrm_iptfs.c:__iptfs_iphlen() { if (iph->version == 0x4) return sizeof(*iph); return sizeof(struct ipv6hdr); } The in-place parser uses the declared header length instead: net/xfrm/xfrm_iptfs.c:__input_process_payload() { ... iplen = be16_to_cpu(iph->tot_len); iphlen = iph->ihl << 2; if (iplen < iphlen || iphlen < sizeof(*iph)) { XFRM_INC_STATS(net, LINUX_MIB_XFRMINHDRERROR); ... } So a runt-started inner IPv4 packet declaring ihl < 5, or ihl * 4 greater than tot_len (for example tot_len 20 with ihl 15), passes the new check, gets a 20 byte allocation, is treated as completely reassembled and is handed to iptfs_complete_inner_skb() and xfrm_input(), while byte-identical input arriving wholly inside one outer payload is rejected with LINUX_MIB_XFRMINHDRERROR. Should the runt path also test iph->ihl so the two parsers agree on both acceptance and MIB accounting? As far as I can tell the residual effect is bounded, since ip_rcv_core() drops such a packet: net/ipv4/ip_input.c:ip_rcv_core() { if (iph->ihl < 5 || iph->version != 4) goto inhdr_error; ... if (!pskb_may_pull(skb, iph->ihl*4)) goto inhdr_error; } The panic class described in the changelog (tot_len in [6, 19] making "ipremain -= copylen" underflow) does look closed by this change for the runt path.
I think this is OK as is. The panic is fixed and it did not add a regression.
Also, the kernel-doc on __iptfs_iphlen() says it returns "the IP header size based on the IP version", and the new comment here says "length has to be at least the IP header size". For IPv4 with options that is the minimum header size rather than the header size. Would it be worth rewording either the comment or the helper's kernel-doc?quoted
XFRM_INC_STATS(xs_net(xtfs->x), LINUX_MIB_XFRMINIPTFSERROR); goto abandon;Two further observations in this same function, both outside the hunk. [Severity: High] This isn't a bug introduced by this patch, but can the copy fallback later in iptfs_reassem_cont() still reach skb_over_panic() on the normal (non-runt) path?
This is not a regression introduced by this patch. Can be fixed as followup.