* [PATCH] psp: reject packets carrying unsupported PSP optional fields
@ 2026-04-30 6:20 David Carlier
2026-04-30 10:32 ` Daniel Zahka
2026-05-01 13:00 ` [PATCH net v2] psp: strip variable-length PSP header in psp_dev_rcv() David Carlier
0 siblings, 2 replies; 9+ messages in thread
From: David Carlier @ 2026-04-30 6:20 UTC (permalink / raw)
To: daniel.zahka, kuba
Cc: willemdebruijn.kernel, davem, edumazet, pabeni, horms, raeds,
kees, cratiu, netdev, linux-kernel, David Carlier, stable
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;
--
2.53.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] psp: reject packets carrying unsupported PSP optional fields
2026-04-30 6:20 [PATCH] psp: reject packets carrying unsupported PSP optional fields David Carlier
@ 2026-04-30 10:32 ` Daniel Zahka
2026-04-30 10:59 ` David CARLIER
2026-05-01 13:00 ` [PATCH net v2] psp: strip variable-length PSP header in psp_dev_rcv() David Carlier
1 sibling, 1 reply; 9+ messages in thread
From: Daniel Zahka @ 2026-04-30 10:32 UTC (permalink / raw)
To: David Carlier, kuba
Cc: willemdebruijn.kernel, davem, edumazet, pabeni, horms, raeds,
kees, cratiu, netdev, linux-kernel, stable
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").
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] psp: reject packets carrying unsupported PSP optional fields
2026-04-30 10:32 ` Daniel Zahka
@ 2026-04-30 10:59 ` David CARLIER
0 siblings, 0 replies; 9+ messages in thread
From: David CARLIER @ 2026-04-30 10:59 UTC (permalink / raw)
To: Daniel Zahka
Cc: kuba, willemdebruijn.kernel, davem, edumazet, pabeni, horms,
raeds, kees, cratiu, netdev, linux-kernel, stable
Hi
> [...] 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.
Makes sense, I'll respin to compute the strip length from
psph->hdrlen (after a second pskb_may_pull to bring the option
bytes in) and drop the rejection -- easier to ignore VC/options
than to refuse them.
> For the other two, I'm not sure they are really necessary.
Will drop both, agreed.
> [...] this function will also need a comment update [...]
Will refresh the kerneldoc.
> For a fix, you'll need to target the net tree with this patch
Ack, will rebase on net for v2.
Thanks,
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH net v2] psp: strip variable-length PSP header in psp_dev_rcv()
2026-04-30 6:20 [PATCH] psp: reject packets carrying unsupported PSP optional fields David Carlier
2026-04-30 10:32 ` Daniel Zahka
@ 2026-05-01 13:00 ` David Carlier
2026-05-01 13:53 ` Willem de Bruijn
` (2 more replies)
1 sibling, 3 replies; 9+ messages in thread
From: David Carlier @ 2026-05-01 13:00 UTC (permalink / raw)
To: daniel.zahka, kuba
Cc: willemdebruijn.kernel, davem, edumazet, pabeni, horms, raeds,
kees, cratiu, netdev, linux-kernel, David Carlier, stable
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 <daniel.zahka@gmail.com>
Cc: stable@vger.kernel.org
Signed-off-by: David Carlier <devnexen@gmail.com>
---
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;
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;
+
+ 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);
--
2.53.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH net v2] psp: strip variable-length PSP header in psp_dev_rcv()
2026-05-01 13:00 ` [PATCH net v2] psp: strip variable-length PSP header in psp_dev_rcv() David Carlier
@ 2026-05-01 13:53 ` Willem de Bruijn
2026-05-01 14:13 ` Daniel Zahka
2026-05-02 0:00 ` Jakub Kicinski
2 siblings, 0 replies; 9+ messages in thread
From: Willem de Bruijn @ 2026-05-01 13:53 UTC (permalink / raw)
To: David Carlier, daniel.zahka, kuba
Cc: willemdebruijn.kernel, davem, edumazet, pabeni, horms, raeds,
kees, cratiu, netdev, linux-kernel, David Carlier, stable
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 <daniel.zahka@gmail.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: David Carlier <devnexen@gmail.com>
Reviewed-by: Willem de Bruijn <willemb@google.com>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net v2] psp: strip variable-length PSP header in psp_dev_rcv()
2026-05-01 13:00 ` [PATCH net v2] psp: strip variable-length PSP header in psp_dev_rcv() David Carlier
2026-05-01 13:53 ` Willem de Bruijn
@ 2026-05-01 14:13 ` Daniel Zahka
2026-05-01 14:39 ` David CARLIER
2026-05-02 0:00 ` Jakub Kicinski
2026-05-02 0:00 ` Jakub Kicinski
2 siblings, 2 replies; 9+ messages in thread
From: Daniel Zahka @ 2026-05-01 14:13 UTC (permalink / raw)
To: David Carlier, kuba
Cc: willemdebruijn.kernel, davem, edumazet, pabeni, horms, raeds,
kees, cratiu, netdev, linux-kernel, stable
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 <daniel.zahka@gmail.com>
No need for the suggested tag here.
> Cc: stable@vger.kernel.org
> Signed-off-by: David Carlier <devnexen@gmail.com>
> ---
> 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 <daniel.zahka@gmail.com>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net v2] psp: strip variable-length PSP header in psp_dev_rcv()
2026-05-01 14:13 ` Daniel Zahka
@ 2026-05-01 14:39 ` David CARLIER
2026-05-02 0:00 ` Jakub Kicinski
1 sibling, 0 replies; 9+ messages in thread
From: David CARLIER @ 2026-05-01 14:39 UTC (permalink / raw)
To: Daniel Zahka
Cc: kuba, willemdebruijn.kernel, davem, edumazet, pabeni, horms,
raeds, kees, cratiu, netdev, linux-kernel, stable
On Fri, 1 May 2026 at 15:13, Daniel Zahka <daniel.zahka@gmail.com> wrote:
>
>
> 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 <daniel.zahka@gmail.com>
>
>
> No need for the suggested tag here.
>
>
> > Cc: stable@vger.kernel.org
> > Signed-off-by: David Carlier <devnexen@gmail.com>
> > ---
> > 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 <daniel.zahka@gmail.com>
>
ACK, will resend tomorrow, Cheers !
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net v2] psp: strip variable-length PSP header in psp_dev_rcv()
2026-05-01 13:00 ` [PATCH net v2] psp: strip variable-length PSP header in psp_dev_rcv() David Carlier
2026-05-01 13:53 ` Willem de Bruijn
2026-05-01 14:13 ` Daniel Zahka
@ 2026-05-02 0:00 ` Jakub Kicinski
2 siblings, 0 replies; 9+ messages in thread
From: Jakub Kicinski @ 2026-05-02 0:00 UTC (permalink / raw)
To: David Carlier
Cc: daniel.zahka, willemdebruijn.kernel, davem, edumazet, pabeni,
horms, raeds, kees, cratiu, netdev, linux-kernel, stable
On Fri, 1 May 2026 14:00:46 +0100 David Carlier wrote:
> + psp_hdr_len = ((u32)psph->hdrlen + 1) * 8;
nit: psp_hlen ? all the other header lengths in this function use hlen
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net v2] psp: strip variable-length PSP header in psp_dev_rcv()
2026-05-01 14:13 ` Daniel Zahka
2026-05-01 14:39 ` David CARLIER
@ 2026-05-02 0:00 ` Jakub Kicinski
1 sibling, 0 replies; 9+ messages in thread
From: Jakub Kicinski @ 2026-05-02 0:00 UTC (permalink / raw)
To: Daniel Zahka
Cc: David Carlier, willemdebruijn.kernel, davem, edumazet, pabeni,
horms, raeds, kees, cratiu, netdev, linux-kernel, stable
On Fri, 1 May 2026 10:13:52 -0400 Daniel Zahka wrote:
> nit: int psp_hlen might be more consistent with the types/names of the
> other local vars.
Ah. :)
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-05-02 0:00 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-30 6:20 [PATCH] psp: reject packets carrying unsupported PSP optional fields David Carlier
2026-04-30 10:32 ` Daniel Zahka
2026-04-30 10:59 ` David CARLIER
2026-05-01 13:00 ` [PATCH net v2] psp: strip variable-length PSP header in psp_dev_rcv() David Carlier
2026-05-01 13:53 ` Willem de Bruijn
2026-05-01 14:13 ` Daniel Zahka
2026-05-01 14:39 ` David CARLIER
2026-05-02 0:00 ` Jakub Kicinski
2026-05-02 0:00 ` Jakub Kicinski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox