[PATCH net-next v5 0/2] net: hsr: strict supervision TLV validation

[PATCH net-next v5 0/2] net: hsr: strict supervision TLV validation

[luka.gejak]

From: Luka Gejak <luka.gejak@linux.dev>

Changes in v5:
 - Reverted TLV loop in Patch 1 to strict sequential parsing per IEC
   62439-3.
 - Retained pskb_may_pull() logic to ensure memory safety for TLV
   headers.
 - Dropped Reviewed-by from Patch 1 due to the logic evolving since
   original review.
 - Added Assisted-by tag for AI-aided translation and formatting to
   both patches.

Changes in v4:
 - Split from a 4-patch series into 'net' and 'net-next' as requested.
 - Implemented a TLV walker in Patch 1 to correctly handle extension
   TLVs and avoid regressions on paged frames/non-linearized skbs.
 - Corrected pskb_may_pull() logic to include the TLV header size.

History of pre-separation series (v1-v3):
Changes in v3:
 - addressed Felix review feedback in the VLAN add unwind fix
 - removed the superfluous empty line

Changes in v2:
 - picked up Reviewed-by tags on patches 1, 3 and 4
 - changes in patch 2 per advice of Felix Maurer

Luka Gejak (2):
  net: hsr: require valid EOT supervision TLV
  net: hsr: reject unresolved interlink ifindex

 net/hsr/hsr_forward.c | 20 +++++++++-----------
 net/hsr/hsr_netlink.c |  7 ++++++-
 2 files changed, 15 insertions(+), 12 deletions(-)

-- 
2.53.0

[PATCH net-next v5 1/2] net: hsr: require valid EOT supervision TLV

[luka.gejak]

From: Luka Gejak <luka.gejak@linux.dev>

Supervision frames are only valid if terminated with a zero-length EOT
TLV. The current check fails to reject non-EOT entries as the terminal
TLV, potentially allowing malformed supervision traffic.

Fix this by strictly requiring the terminal TLV to be HSR_TLV_EOT
with a length of zero, and properly linearizing the TLV header before
access.

Assisted-by: Gemini:Gemini-3.1-flash
Signed-off-by: Luka Gejak <luka.gejak@linux.dev>
---
 net/hsr/hsr_forward.c | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/net/hsr/hsr_forward.c b/net/hsr/hsr_forward.c
index 0aca859c88cb..eb89cc44eac0 100644
--- a/net/hsr/hsr_forward.c
+++ b/net/hsr/hsr_forward.c
@@ -82,35 +82,33 @@ static bool is_supervision_frame(struct hsr_priv *hsr, struct sk_buff *skb)
 	    hsr_sup_tag->tlv.HSR_TLV_length != sizeof(struct hsr_sup_payload))
 		return false;
 
-	/* Get next tlv */
+	/* Get next TLV */
 	total_length += hsr_sup_tag->tlv.HSR_TLV_length;
-	if (!pskb_may_pull(skb, total_length))
+	if (!pskb_may_pull(skb, total_length + sizeof(struct hsr_sup_tlv)))
 		return false;
 	skb_pull(skb, total_length);
 	hsr_sup_tlv = (struct hsr_sup_tlv *)skb->data;
 	skb_push(skb, total_length);
 
-	/* if this is a redbox supervision frame we need to verify
-	 * that more data is available
-	 */
+	/* If this is a RedBox supervision frame, verify additional data */
 	if (hsr_sup_tlv->HSR_TLV_type == PRP_TLV_REDBOX_MAC) {
-		/* tlv length must be a length of a mac address */
+		/* TLV length must be the size of a MAC address */
 		if (hsr_sup_tlv->HSR_TLV_length != sizeof(struct hsr_sup_payload))
 			return false;
 
-		/* make sure another tlv follows */
+		/* Make sure another TLV follows */
 		total_length += sizeof(struct hsr_sup_tlv) + hsr_sup_tlv->HSR_TLV_length;
-		if (!pskb_may_pull(skb, total_length))
+		if (!pskb_may_pull(skb, total_length + sizeof(struct hsr_sup_tlv)))
 			return false;
 
-		/* get next tlv */
+		/* Get next TLV */
 		skb_pull(skb, total_length);
 		hsr_sup_tlv = (struct hsr_sup_tlv *)skb->data;
 		skb_push(skb, total_length);
 	}
 
-	/* end of tlvs must follow at the end */
-	if (hsr_sup_tlv->HSR_TLV_type == HSR_TLV_EOT &&
+	/* Supervision frame must end with EOT TLV */
+	if (hsr_sup_tlv->HSR_TLV_type != HSR_TLV_EOT ||
 	    hsr_sup_tlv->HSR_TLV_length != 0)
 		return false;
 
-- 
2.53.0

[PATCH net-next v5 2/2] net: hsr: reject unresolved interlink ifindex

[luka.gejak]

From: Luka Gejak <luka.gejak@linux.dev>

In hsr_newlink(), a provided but invalid IFLA_HSR_INTERLINK attribute
was silently ignored if __dev_get_by_index() returned NULL. This leads
to incorrect RedBox topology creation without notifying the user.

Fix this by returning -EINVAL and an extack message when the
interlink attribute is present but cannot be resolved.

Assisted-by: Gemini:Gemini-3.1-flash
Reviewed-by: Felix Maurer <fmaurer@redhat.com>
Signed-off-by: Luka Gejak <luka.gejak@linux.dev>
---
 net/hsr/hsr_netlink.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/net/hsr/hsr_netlink.c b/net/hsr/hsr_netlink.c
index db0b0af7a692..f0ca23da3ab9 100644
--- a/net/hsr/hsr_netlink.c
+++ b/net/hsr/hsr_netlink.c
@@ -76,9 +76,14 @@ static int hsr_newlink(struct net_device *dev,
 		return -EINVAL;
 	}
 
-	if (data[IFLA_HSR_INTERLINK])
+	if (data[IFLA_HSR_INTERLINK]) {
 		interlink = __dev_get_by_index(link_net,
 					       nla_get_u32(data[IFLA_HSR_INTERLINK]));
+		if (!interlink) {
+			NL_SET_ERR_MSG_MOD(extack, "Interlink does not exist");
+			return -EINVAL;
+		}
+	}
 
 	if (interlink && interlink == link[0]) {
 		NL_SET_ERR_MSG_MOD(extack, "Interlink and Slave1 are the same");
-- 
2.53.0

Re: [PATCH net-next v5 1/2] net: hsr: require valid EOT supervision TLV

[Fernando Fernandez Mancera]

On 4/7/26 6:25 PM, luka.gejak@linux.dev wrote:
> From: Luka Gejak <luka.gejak@linux.dev>
> 
> Supervision frames are only valid if terminated with a zero-length EOT
> TLV. The current check fails to reject non-EOT entries as the terminal
> TLV, potentially allowing malformed supervision traffic.
> 
> Fix this by strictly requiring the terminal TLV to be HSR_TLV_EOT
> with a length of zero, and properly linearizing the TLV header before
> access.
> 
> Assisted-by: Gemini:Gemini-3.1-flash
> Signed-off-by: Luka Gejak <luka.gejak@linux.dev>
> ---
>   net/hsr/hsr_forward.c | 20 +++++++++-----------
>   1 file changed, 9 insertions(+), 11 deletions(-)
> 
> diff --git a/net/hsr/hsr_forward.c b/net/hsr/hsr_forward.c
> index 0aca859c88cb..eb89cc44eac0 100644
> --- a/net/hsr/hsr_forward.c
> +++ b/net/hsr/hsr_forward.c
> @@ -82,35 +82,33 @@ static bool is_supervision_frame(struct hsr_priv *hsr, struct sk_buff *skb)
>   	    hsr_sup_tag->tlv.HSR_TLV_length != sizeof(struct hsr_sup_payload))
>   		return false;
>   
> -	/* Get next tlv */
> +	/* Get next TLV */
>   	total_length += hsr_sup_tag->tlv.HSR_TLV_length;
> -	if (!pskb_may_pull(skb, total_length))
> +	if (!pskb_may_pull(skb, total_length + sizeof(struct hsr_sup_tlv)))
>   		return false;
>   	skb_pull(skb, total_length);
>   	hsr_sup_tlv = (struct hsr_sup_tlv *)skb->data;
>   	skb_push(skb, total_length);
>   
> -	/* if this is a redbox supervision frame we need to verify
> -	 * that more data is available
> -	 */
> +	/* If this is a RedBox supervision frame, verify additional data */
>   	if (hsr_sup_tlv->HSR_TLV_type == PRP_TLV_REDBOX_MAC) {
> -		/* tlv length must be a length of a mac address */
> +		/* TLV length must be the size of a MAC address */
>   		if (hsr_sup_tlv->HSR_TLV_length != sizeof(struct hsr_sup_payload))
>   			return false;
>   
> -		/* make sure another tlv follows */
> +		/* Make sure another TLV follows */
>   		total_length += sizeof(struct hsr_sup_tlv) + hsr_sup_tlv->HSR_TLV_length;
> -		if (!pskb_may_pull(skb, total_length))
> +		if (!pskb_may_pull(skb, total_length + sizeof(struct hsr_sup_tlv)))

Hi Luka,

why is the length adjusted here? The current total length was adjusted 
correctly above see:

total_length += sizeof(struct hsr_sup_tlv) + hsr_sup_tlv->HSR_TLV_length;

Thanks,
Fernando.

>   			return false;
>   
> -		/* get next tlv */
> +		/* Get next TLV */
>   		skb_pull(skb, total_length);
>   		hsr_sup_tlv = (struct hsr_sup_tlv *)skb->data;
>   		skb_push(skb, total_length);
>   	}
>   
> -	/* end of tlvs must follow at the end */
> -	if (hsr_sup_tlv->HSR_TLV_type == HSR_TLV_EOT &&
> +	/* Supervision frame must end with EOT TLV */
> +	if (hsr_sup_tlv->HSR_TLV_type != HSR_TLV_EOT ||
>   	    hsr_sup_tlv->HSR_TLV_length != 0)
>   		return false;
>   

Re: [PATCH net-next v5 1/2] net: hsr: require valid EOT supervision TLV

[Fernando Fernandez Mancera]

On 4/8/26 9:48 AM, Fernando Fernandez Mancera wrote:
> On 4/7/26 6:25 PM, luka.gejak@linux.dev wrote:
>> From: Luka Gejak <luka.gejak@linux.dev>
>>
>> Supervision frames are only valid if terminated with a zero-length EOT
>> TLV. The current check fails to reject non-EOT entries as the terminal
>> TLV, potentially allowing malformed supervision traffic.
>>
>> Fix this by strictly requiring the terminal TLV to be HSR_TLV_EOT
>> with a length of zero, and properly linearizing the TLV header before
>> access.
>>
>> Assisted-by: Gemini:Gemini-3.1-flash
>> Signed-off-by: Luka Gejak <luka.gejak@linux.dev>
>> ---
>>   net/hsr/hsr_forward.c | 20 +++++++++-----------
>>   1 file changed, 9 insertions(+), 11 deletions(-)
>>
>> diff --git a/net/hsr/hsr_forward.c b/net/hsr/hsr_forward.c
>> index 0aca859c88cb..eb89cc44eac0 100644
>> --- a/net/hsr/hsr_forward.c
>> +++ b/net/hsr/hsr_forward.c
>> @@ -82,35 +82,33 @@ static bool is_supervision_frame(struct hsr_priv 
>> *hsr, struct sk_buff *skb)
>>           hsr_sup_tag->tlv.HSR_TLV_length != sizeof(struct 
>> hsr_sup_payload))
>>           return false;
>> -    /* Get next tlv */
>> +    /* Get next TLV */
>>       total_length += hsr_sup_tag->tlv.HSR_TLV_length;
>> -    if (!pskb_may_pull(skb, total_length))
>> +    if (!pskb_may_pull(skb, total_length + sizeof(struct hsr_sup_tlv)))
>>           return false;
>>       skb_pull(skb, total_length);
>>       hsr_sup_tlv = (struct hsr_sup_tlv *)skb->data;
>>       skb_push(skb, total_length);
>> -    /* if this is a redbox supervision frame we need to verify
>> -     * that more data is available
>> -     */
>> +    /* If this is a RedBox supervision frame, verify additional data */
>>       if (hsr_sup_tlv->HSR_TLV_type == PRP_TLV_REDBOX_MAC) {
>> -        /* tlv length must be a length of a mac address */
>> +        /* TLV length must be the size of a MAC address */
>>           if (hsr_sup_tlv->HSR_TLV_length != sizeof(struct 
>> hsr_sup_payload))
>>               return false;
>> -        /* make sure another tlv follows */
>> +        /* Make sure another TLV follows */
>>           total_length += sizeof(struct hsr_sup_tlv) + hsr_sup_tlv- 
>> >HSR_TLV_length;
>> -        if (!pskb_may_pull(skb, total_length))
>> +        if (!pskb_may_pull(skb, total_length + sizeof(struct 
>> hsr_sup_tlv)))
> 
> Hi Luka,
> 
> why is the length adjusted here? The current total length was adjusted 
> correctly above see:
> 

Never mind, it makes sense as the total_length must point to the 
beginning of the next TLV.

LGTM,

Reviewed-by: Fernando Fernandez Mancera <fmancera@suse.de>

Thanks,
Fernando.

>>               return false;
>> -        /* get next tlv */
>> +        /* Get next TLV */
>>           skb_pull(skb, total_length);
>>           hsr_sup_tlv = (struct hsr_sup_tlv *)skb->data;
>>           skb_push(skb, total_length);
>>       }
>> -    /* end of tlvs must follow at the end */
>> -    if (hsr_sup_tlv->HSR_TLV_type == HSR_TLV_EOT &&
>> +    /* Supervision frame must end with EOT TLV */
>> +    if (hsr_sup_tlv->HSR_TLV_type != HSR_TLV_EOT ||
>>           hsr_sup_tlv->HSR_TLV_length != 0)
>>           return false;
> 

Re: [PATCH net-next v5 1/2] net: hsr: require valid EOT supervision TLV

[Luka Gejak]

On April 8, 2026 10:05:31 AM GMT+02:00, Fernando Fernandez Mancera <fmancera@suse.de> wrote:
>On 4/8/26 9:48 AM, Fernando Fernandez Mancera wrote:
>> On 4/7/26 6:25 PM, luka.gejak@linux.dev wrote:
>>> From: Luka Gejak <luka.gejak@linux.dev>
>>> 
>>> Supervision frames are only valid if terminated with a zero-length EOT
>>> TLV. The current check fails to reject non-EOT entries as the terminal
>>> TLV, potentially allowing malformed supervision traffic.
>>> 
>>> Fix this by strictly requiring the terminal TLV to be HSR_TLV_EOT
>>> with a length of zero, and properly linearizing the TLV header before
>>> access.
>>> 
>>> Assisted-by: Gemini:Gemini-3.1-flash
>>> Signed-off-by: Luka Gejak <luka.gejak@linux.dev>
>>> ---
>>>   net/hsr/hsr_forward.c | 20 +++++++++-----------
>>>   1 file changed, 9 insertions(+), 11 deletions(-)
>>> 
>>> diff --git a/net/hsr/hsr_forward.c b/net/hsr/hsr_forward.c
>>> index 0aca859c88cb..eb89cc44eac0 100644
>>> --- a/net/hsr/hsr_forward.c
>>> +++ b/net/hsr/hsr_forward.c
>>> @@ -82,35 +82,33 @@ static bool is_supervision_frame(struct hsr_priv *hsr, struct sk_buff *skb)
>>>           hsr_sup_tag->tlv.HSR_TLV_length != sizeof(struct hsr_sup_payload))
>>>           return false;
>>> -    /* Get next tlv */
>>> +    /* Get next TLV */
>>>       total_length += hsr_sup_tag->tlv.HSR_TLV_length;
>>> -    if (!pskb_may_pull(skb, total_length))
>>> +    if (!pskb_may_pull(skb, total_length + sizeof(struct hsr_sup_tlv)))
>>>           return false;
>>>       skb_pull(skb, total_length);
>>>       hsr_sup_tlv = (struct hsr_sup_tlv *)skb->data;
>>>       skb_push(skb, total_length);
>>> -    /* if this is a redbox supervision frame we need to verify
>>> -     * that more data is available
>>> -     */
>>> +    /* If this is a RedBox supervision frame, verify additional data */
>>>       if (hsr_sup_tlv->HSR_TLV_type == PRP_TLV_REDBOX_MAC) {
>>> -        /* tlv length must be a length of a mac address */
>>> +        /* TLV length must be the size of a MAC address */
>>>           if (hsr_sup_tlv->HSR_TLV_length != sizeof(struct hsr_sup_payload))
>>>               return false;
>>> -        /* make sure another tlv follows */
>>> +        /* Make sure another TLV follows */
>>>           total_length += sizeof(struct hsr_sup_tlv) + hsr_sup_tlv- >HSR_TLV_length;
>>> -        if (!pskb_may_pull(skb, total_length))
>>> +        if (!pskb_may_pull(skb, total_length + sizeof(struct hsr_sup_tlv)))
>> 
>> Hi Luka,
>> 
>> why is the length adjusted here? The current total length was adjusted correctly above see:
>> 
>
>Never mind, it makes sense as the total_length must point to the beginning of the next TLV.
>
>LGTM,
>
>Reviewed-by: Fernando Fernandez Mancera <fmancera@suse.de>
>
>Thanks,
>Fernando.
>
Hi Fernando,
Thank you for the follow-up and the reviewed-by tag.
I was actually just about to send an explanation regarding the 
pskb_may_pull() logic, but that seems unnecessary now. I'm glad it 
makes sense on your end as well.
Best regards,
Luka
>>>               return false;
>>> -        /* get next tlv */
>>> +        /* Get next TLV */
>>>           skb_pull(skb, total_length);
>>>           hsr_sup_tlv = (struct hsr_sup_tlv *)skb->data;
>>>           skb_push(skb, total_length);
>>>       }
>>> -    /* end of tlvs must follow at the end */
>>> -    if (hsr_sup_tlv->HSR_TLV_type == HSR_TLV_EOT &&
>>> +    /* Supervision frame must end with EOT TLV */
>>> +    if (hsr_sup_tlv->HSR_TLV_type != HSR_TLV_EOT ||
>>>           hsr_sup_tlv->HSR_TLV_length != 0)
>>>           return false;
>> 
>

Re: [PATCH net-next v5 1/2] net: hsr: require valid EOT supervision TLV

[Felix Maurer]

On Tue, Apr 07, 2026 at 06:25:01PM +0200, luka.gejak@linux.dev wrote:
> From: Luka Gejak <luka.gejak@linux.dev>
>
> Supervision frames are only valid if terminated with a zero-length EOT
> TLV. The current check fails to reject non-EOT entries as the terminal
> TLV, potentially allowing malformed supervision traffic.
>
> Fix this by strictly requiring the terminal TLV to be HSR_TLV_EOT
> with a length of zero, and properly linearizing the TLV header before
> access.
>
> Assisted-by: Gemini:Gemini-3.1-flash
> Signed-off-by: Luka Gejak <luka.gejak@linux.dev>

Thanks for the patch, including making sure that there is enough data
available to read the next TLVs.

Reviewed-by: Felix Maurer <fmaurer@redhat.com>

> ---
>  net/hsr/hsr_forward.c | 20 +++++++++-----------
>  1 file changed, 9 insertions(+), 11 deletions(-)
>
> diff --git a/net/hsr/hsr_forward.c b/net/hsr/hsr_forward.c
> index 0aca859c88cb..eb89cc44eac0 100644
> --- a/net/hsr/hsr_forward.c
> +++ b/net/hsr/hsr_forward.c
> @@ -82,35 +82,33 @@ static bool is_supervision_frame(struct hsr_priv *hsr, struct sk_buff *skb)
>  	    hsr_sup_tag->tlv.HSR_TLV_length != sizeof(struct hsr_sup_payload))
>  		return false;
>
> -	/* Get next tlv */
> +	/* Get next TLV */
>  	total_length += hsr_sup_tag->tlv.HSR_TLV_length;
> -	if (!pskb_may_pull(skb, total_length))
> +	if (!pskb_may_pull(skb, total_length + sizeof(struct hsr_sup_tlv)))
>  		return false;
>  	skb_pull(skb, total_length);
>  	hsr_sup_tlv = (struct hsr_sup_tlv *)skb->data;
>  	skb_push(skb, total_length);
>
> -	/* if this is a redbox supervision frame we need to verify
> -	 * that more data is available
> -	 */
> +	/* If this is a RedBox supervision frame, verify additional data */
>  	if (hsr_sup_tlv->HSR_TLV_type == PRP_TLV_REDBOX_MAC) {
> -		/* tlv length must be a length of a mac address */
> +		/* TLV length must be the size of a MAC address */
>  		if (hsr_sup_tlv->HSR_TLV_length != sizeof(struct hsr_sup_payload))
>  			return false;
>
> -		/* make sure another tlv follows */
> +		/* Make sure another TLV follows */
>  		total_length += sizeof(struct hsr_sup_tlv) + hsr_sup_tlv->HSR_TLV_length;
> -		if (!pskb_may_pull(skb, total_length))
> +		if (!pskb_may_pull(skb, total_length + sizeof(struct hsr_sup_tlv)))
>  			return false;
>
> -		/* get next tlv */
> +		/* Get next TLV */
>  		skb_pull(skb, total_length);
>  		hsr_sup_tlv = (struct hsr_sup_tlv *)skb->data;
>  		skb_push(skb, total_length);
>  	}
>
> -	/* end of tlvs must follow at the end */
> -	if (hsr_sup_tlv->HSR_TLV_type == HSR_TLV_EOT &&
> +	/* Supervision frame must end with EOT TLV */
> +	if (hsr_sup_tlv->HSR_TLV_type != HSR_TLV_EOT ||
>  	    hsr_sup_tlv->HSR_TLV_length != 0)
>  		return false;
>
> --
> 2.53.0
>