From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-qt1-f180.google.com (mail-qt1-f180.google.com [209.85.160.180]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id EBA403C13E6 for ; Fri, 1 May 2026 14:13:54 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.160.180 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1777644836; cv=none; b=nrWFK96beNbxLVAK6p+Ggw5S4BBmXDcyxHMjUMUMmvdUdK+xVUiov6vym0VBT5Y0BgIedrb8x0b5wyzHNmSJAoJtXuzE1ag7GEPIymdYyG3mjlroLeqFYbBYYCWhPJoNnm2Ol4+Gk1Pa81/81RGDLcNQARlHit4AuA+YdumjTgA= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1777644836; c=relaxed/simple; bh=09xIVSIOCHld7sNJ6+3WxJZmKrRnpZPPPMGuyfUUpXs=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=ciR8L+WlV5L7jHNr01ypaQWR9GcHRMGfS9tFyDmRmqItHiKrw+F8j1G3WT1MArki7YrWdY0rA/ANRUq7GxpKp/yVgl2gnhCU1pNuHQJj24GFAH0TbXkozCu1HbuHnB9KcNSgemMGSlfKDDUZ2WjA/NTjFaLae19HJsVxOpIf+N8= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=fByLrzER; arc=none smtp.client-ip=209.85.160.180 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="fByLrzER" Received: by mail-qt1-f180.google.com with SMTP id d75a77b69052e-50d880e6fbbso28131161cf.0 for ; Fri, 01 May 2026 07:13:54 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1777644834; x=1778249634; darn=vger.kernel.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=fmbjUGxD1u3NABfRek7G1d5V6wG4/EqTR/sOvz6Wzxg=; b=fByLrzERHPfMgWyirobyM1H2k5Dsh8sXkVm1z8Cfjiid25AE5CFrypKXvTkYUjuOF3 dGCX0Sev6iB2MgwfFLHvxlePIhX4wZQgnsdI9Eb1F44iX14U2Gc/TTB5YM6sxjCKb8TI tYc4tWaMEvohFFKkV0wrMJtGmScOk6txc9iyZWqdLAKq0h1JUzkiSNKavpPNUjFWEUBS OKVJsHoDWRdK0fJX6VYCBx8fvkb1ZNSGVRK6fIGg4Loh28+oz3IMsiSTS4ndRpZuP9r3 1DGffyiu8kG1j4phxluZBG3awNpAfqPBp8TwqAt0eMmA4RMzSXI+PP/y3BDajxQUD3jg uobw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1777644834; x=1778249634; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=fmbjUGxD1u3NABfRek7G1d5V6wG4/EqTR/sOvz6Wzxg=; b=Hq93DAh8MaIt+MFHgbFngFlk3Vo7krXTPWfqL+OZM71Y2N0WSrn3OhSn0J2heyTiJY 8Nk96VUN1DZ31BLjsNaeBCKHc1SPQAKYXDPYSBr7zwHajtp1bJ4x4yj9jQ8fYVR3SxYp b7jrlAV+DmrxQh+lX26g7wZg9sGTmziU+XdoN05Yf1X/PCMcYr9N8Oko1WVeIvjW2XZt YP/Ww7IevzST5WtPxWNPGSz6GnIJGk6YlcMqwk9ngNiRJKdhGtOxX+oqLyCUgAk58sgP 0YlI46tGErX2Dg5z7hk9pSzgTFFh9CRAm13Enjhq7WDhl6iwqe7CNluB/T9KjTFM8NUS +ADQ== X-Forwarded-Encrypted: i=1; AFNElJ9RetscICtBRzEcLWW3UECHcPQwDCZSBhX5ewr86YmAHykNaE+ZpV+5+JfriPxF23JNrKEq5PEd4ptXBZw=@vger.kernel.org X-Gm-Message-State: AOJu0YwT+gJzFbeAI0FWXBXSUDqBJLIBR9AVvHpMIZqcOkg3Uxvt9tx4 rLUGPwkmVYdvojlzU2dOigqtgnzgM1ZY6NC7xc9H9zjE5ac2aEUWshwTXrWC0w== X-Gm-Gg: AeBDieti71HeHLsZMljnzv30YdqeGYJ5hBdCk08n5Y//NDKCi3Hx5/vILnDKxKokY5L iyCcRp4o0Qd0BSaKbgmTJVrkTV6Mp5q5+vSg68OQwhYfu6AsEEvnmrVkr7zFUz1Ncc5urLoVees LEJIsa0nqiDcbdWIMrsNKIjbzgVhhnC2M46vp6cudH6H3F/qSVsYzRCFbBPM7aaJxXNGkSYbwx8 Er1Zg84NteQ0UYJ0D2meURBVYciu6u3aKiiKGq2xnATDnwRMqLuIsSQ+ScG8Aj3FCqRjWD6kMsE E2I1tqEWX/DSPCnc9+vvDAPpk9+7s3PXgbQJUCBdtPL1EqDYMN7kRj/TKCUur/udUBrihxBhBRh jEMu4Nebgl+/tae1h/VBZke4ppo14lS4rCX9WspD1ctEqthWAMzY4K9ZAc03GRvdxhmToPRjPv8 vYBxa2NBvzEpOCWUSPsLCR9Ki/YiSteOFlztBuQfNSPA0RSJ18JHkqYAELK5STviEP9jTC+HxUd kSBgBz7FGrh22sykThj5IXL X-Received: by 2002:ac8:5d14:0:b0:50e:18f9:b5e2 with SMTP id d75a77b69052e-5102d09af36mr91940941cf.6.1777644833795; Fri, 01 May 2026 07:13:53 -0700 (PDT) Received: from ?IPV6:2a03:83e0:1145:4:7cf5:7b4:6072:d3b2? ([2620:10d:c091:500::3:18f8]) by smtp.gmail.com with ESMTPSA id d75a77b69052e-51040b86535sm17194531cf.27.2026.05.01.07.13.52 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 01 May 2026 07:13:53 -0700 (PDT) Message-ID: Date: Fri, 1 May 2026 10:13:52 -0400 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH net v2] psp: strip variable-length PSP header in psp_dev_rcv() To: David Carlier , kuba@kernel.org Cc: willemdebruijn.kernel@gmail.com, davem@davemloft.net, edumazet@google.com, pabeni@redhat.com, horms@kernel.org, raeds@nvidia.com, kees@kernel.org, cratiu@nvidia.com, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, stable@vger.kernel.org References: <20260430062033.20428-1-devnexen@gmail.com> <20260501130046.16008-1-devnexen@gmail.com> Content-Language: en-US From: Daniel Zahka In-Reply-To: <20260501130046.16008-1-devnexen@gmail.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 5/1/26 9:00 AM, David Carlier wrote: > psp_dev_rcv() unconditionally removes a fixed PSP_ENCAP_HLEN, even > when psph->hdrlen indicates that the PSP header carries optional > fields. A frame whose PSP header advertises a non-zero VC or any > extension would therefore be silently mis-decapsulated: option bytes > would spill into the inner packet head and downstream parsing would > fail on a corrupted skb. > > Compute the full PSP header length from psph->hdrlen, pull the > optional bytes into the linear region, and strip the whole header > when decapsulating. Optional fields (VC, ...) are still ignored, > just discarded with the rest of the header instead of leaking. > crypt_offset and the VIRT flag are intentionally not validated here > - callers know their device's PSP implementation and can decide. > > Both in-tree callers gate on hardware-validated PSP, so this is a > correctness fix rather than a reachable corruption path under > current configurations. > > Fixes: 0eddb8023cee ("psp: provide decapsulation and receive helper for drivers") > Suggested-by: Daniel Zahka No need for the suggested tag here. > Cc: stable@vger.kernel.org > Signed-off-by: David Carlier > --- > v1 -> v2 (per Daniel Zahka): > - strip the variable-length PSP header (psph->hdrlen) instead of > rejecting opt-bearing frames; VC/options are ignored, not refused > - drop the crypt_offset and PSPHDR_VERFL_VIRT checks > - refresh kerneldoc above psp_dev_rcv() > - retarget at net (was net-next) > > net/psp/psp_main.c | 41 +++++++++++++++++++++++++++++++---------- > 1 file changed, 31 insertions(+), 10 deletions(-) > > diff --git a/net/psp/psp_main.c b/net/psp/psp_main.c > index 9508b6c38003..b040345d7273 100644 > --- a/net/psp/psp_main.c > +++ b/net/psp/psp_main.c > @@ -263,15 +263,17 @@ EXPORT_SYMBOL(psp_dev_encapsulate); > > /* Receive handler for PSP packets. > * > - * Presently it accepts only already-authenticated packets and does not > - * support optional fields, such as virtualization cookies. The caller should > - * ensure that skb->data is pointing to the mac header, and that skb->mac_len > - * is set. This function does not currently adjust skb->csum (CHECKSUM_COMPLETE > - * is not supported). > + * Accepts only already-authenticated packets. The full PSP header is > + * stripped according to psph->hdrlen; any optional fields it advertises > + * (virtualization cookies, etc.) are ignored and discarded along with the > + * rest of the header. The caller should ensure that skb->data is pointing > + * to the mac header, and that skb->mac_len is set. This function does not > + * currently adjust skb->csum (CHECKSUM_COMPLETE is not supported). > */ > int psp_dev_rcv(struct sk_buff *skb, u16 dev_id, u8 generation, bool strip_icv) > { > int l2_hlen = 0, l3_hlen, encap; > + u32 psp_hdr_len; There is a style convention in the networking subsystem that declarations are sorted longest to shortest from top to bottom. Let's maintain that here. nit: int psp_hlen might be more consistent with the types/names of the other local vars. > struct psp_skb_ext *pse; > struct psphdr *psph; > struct ethhdr *eth; > @@ -312,18 +314,36 @@ int psp_dev_rcv(struct sk_buff *skb, u16 dev_id, u8 generation, bool strip_icv) > if (unlikely(uh->dest != htons(PSP_DEFAULT_UDP_PORT))) > return -EINVAL; > > - pse = skb_ext_add(skb, SKB_EXT_PSP); > - if (!pse) > + psph = (struct psphdr *)(skb->data + l2_hlen + l3_hlen + > + sizeof(struct udphdr)); > + > + /* Strip the full PSP header per psph->hdrlen; VC/options are pulled > + * into the linear region only so they can be discarded with the > + * rest of the header. > + */ > + psp_hdr_len = ((u32)psph->hdrlen + 1) * 8; I don't believe casting psph->hdrlen to u32 is necessary for correctness here. > + > + if (unlikely(psp_hdr_len < sizeof(struct psphdr))) > + return -EINVAL; > + > + if (psp_hdr_len > sizeof(struct psphdr) && > + !pskb_may_pull(skb, l2_hlen + l3_hlen + > + sizeof(struct udphdr) + psp_hdr_len)) > return -EINVAL; > > psph = (struct psphdr *)(skb->data + l2_hlen + l3_hlen + > sizeof(struct udphdr)); > + > + pse = skb_ext_add(skb, SKB_EXT_PSP); > + if (!pse) > + return -EINVAL; > + > pse->spi = psph->spi; > pse->dev_id = dev_id; > pse->generation = generation; > pse->version = FIELD_GET(PSPHDR_VERFL_VERSION, psph->verfl); > > - encap = PSP_ENCAP_HLEN; > + encap = sizeof(struct udphdr) + psp_hdr_len; > encap += strip_icv ? PSP_TRL_SIZE : 0; > > if (proto == htons(ETH_P_IP)) { > @@ -340,8 +360,9 @@ int psp_dev_rcv(struct sk_buff *skb, u16 dev_id, u8 generation, bool strip_icv) > ipv6h->payload_len = htons(ntohs(ipv6h->payload_len) - encap); > } > > - memmove(skb->data + PSP_ENCAP_HLEN, skb->data, l2_hlen + l3_hlen); > - skb_pull(skb, PSP_ENCAP_HLEN); > + memmove(skb->data + sizeof(struct udphdr) + psp_hdr_len, > + skb->data, l2_hlen + l3_hlen); > + skb_pull(skb, sizeof(struct udphdr) + psp_hdr_len); > > if (strip_icv) > pskb_trim(skb, skb->len - PSP_TRL_SIZE); Minor comments, but otherwise lgtm. Reviewed-by: Daniel Zahka