1. Patchew
  2. linux
  3. View series

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

David Carlier posted 1 patch 1 month, 2 weeks ago 
net/psp/psp_main.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
[PATCH] psp: reject packets carrying unsupported PSP optional fields
Posted by David Carlier 1 month, 2 weeks ago 
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
Re: [PATCH] psp: reject packets carrying unsupported PSP optional fields
Posted by Daniel Zahka 1 month, 2 weeks ago 
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").
Re: [PATCH] psp: reject packets carrying unsupported PSP optional fields
Posted by David CARLIER 1 month, 2 weeks ago 
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,
[PATCH net v2] psp: strip variable-length PSP header in psp_dev_rcv()
Posted by David Carlier 1 month, 2 weeks ago 
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
Re: [PATCH net v2] psp: strip variable-length PSP header in psp_dev_rcv()
Posted by Jakub Kicinski 1 month, 2 weeks ago 
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
Re: [PATCH net v2] psp: strip variable-length PSP header in psp_dev_rcv()
Posted by Daniel Zahka 1 month, 2 weeks ago 
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>
Re: [PATCH net v2] psp: strip variable-length PSP header in psp_dev_rcv()
Posted by Jakub Kicinski 1 month, 2 weeks ago 
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. :)
Re: [PATCH net v2] psp: strip variable-length PSP header in psp_dev_rcv()
Posted by David CARLIER 1 month, 2 weeks ago 
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 !
Re: [PATCH net v2] psp: strip variable-length PSP header in psp_dev_rcv()
Posted by Willem de Bruijn 1 month, 2 weeks ago 
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>

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.