Re: [PATCH] psp: reject packets carrying unsupported PSP optional fields

Daniel Zahka
2026-04-30T10:32:23Z

On 4/30/26 2:20 AM, David Carlier wrote:
> psp_dev_rcv() documents that it does not support optional PSP fields
> but never enforces it. The helper unconditionally strips a fixed
> PSP_ENCAP_HLEN, so a frame whose PSP header carries options is
> silently mis-decapsulated: option bytes spill into the inner packet
> head and parsing fails downstream on a corrupted skb instead of being
> rejected early.
>
> Validate hdrlen, crypt_offset and PSPHDR_VERFL_VIRT, and hoist the
> psph read above skb_ext_add() so rejected packets do not pick up an
> SKB_EXT_PSP extension only to drop it. Both in-tree callers gate on
> hardware-validated, opt-less PSP, so this is hardening rather than a
> reachable corruption path.
>
> Fixes: 0eddb8023cee ("psp: provide decapsulation and receive helper for drivers")
> Cc: stable@vger.kernel.org
> Signed-off-by: David Carlier <devnexen@gmail.com>
> ---
> net/psp/psp_main.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/net/psp/psp_main.c b/net/psp/psp_main.c
> index 524978dfb8fd..53d7e14c054a 100644
> --- a/net/psp/psp_main.c
> +++ b/net/psp/psp_main.c
> @@ -321,12 +321,20 @@ 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;
>
> + psph = (struct psphdr *)(skb->data + l2_hlen + l3_hlen +
> + sizeof(struct udphdr));
> +
> + /* Fixed-length decap; reject optional fields rather than mis-decapsulate. */
> +
> + if (unlikely(psph->hdrlen != PSP_HDRLEN_NOOPT ||
> + psph->crypt_offset ||
> + (psph->verfl & PSPHDR_VERFL_VIRT)))
> + return -EINVAL;
> +
> pse = skb_ext_add(skb, SKB_EXT_PSP);
> if (!pse)
> return -EINVAL;
>
> - psph = (struct psphdr *)(skb->data + l2_hlen + l3_hlen +
> - sizeof(struct udphdr));
> pse->spi = psph->spi;
> pse->dev_id = dev_id;
> pse->generation = generation;

Thanks. There is a bit of a gray area in regards to how much we need to
validate here, because callers ought to use this only downstream of a
real PSP device, and only when that device has emitted some metadata
that the skb was at least parsed as PSP and decrypted correctly. That
being said, the handling of psp hdrlen is definitely a bug, because even
valid values can cause corruption during decap as you noted. I think we
should fix it by validating that it is less than the remaining bytes
after the psp-udp header, and then stripping the correct header length
accordingly.

For the other two, I'm not sure they are really necessary. Mandating
that crypt off is 0 is too restrictive as this function should work just
fine with non-zero values. We could validate that it isn't too long or
misaligned with the payload, but it may be better to have callers know
their NICs PSP implementation and decide if that is necessary. Similar
story with the VC present bit.

In any case, I think this function will also need a comment update
giving some more context for caller, and we can mention that the whole
psp header will be stripped away according the psp hdrlen, and that any
vc or options will be ignored and discarded.

For a fix, you'll need to target the net tree with this patch, (i.e.
"PATCH net").