openvswitch: reject a negative transport length in the IPv6 L4 checksum update

[PATCH net] openvswitch: reject a negative transport length in the IPv6 L4 checksum update

Posted by Norbert Szetei 1 week, 2 days ago

update_ipv6_checksum() computes

	int transport_len = skb->len - skb_transport_offset(skb);

and gates the L4 access on

	if (likely(transport_len >= sizeof(struct tcphdr)))

sizeof yields size_t, so transport_len is converted to unsigned and every
negative value passes the guard.  inet_proto_csum_replace16() then reads
the transport checksum field out of bounds, and writes the updated value
back to the same address:

  BUG: KASAN: slab-use-after-free in inet_proto_csum_replace16+0x445/0x470
  Read of size 2 at addr ffff888131a4cb06 by task ovs_ipv6_oob/696
  CPU: 3 UID: 1000 PID: 696 Comm: ovs_ipv6_oob Tainted: G N 7.3.0-rc2+ #338
  Call Trace:
   inet_proto_csum_replace16+0x445/0x470
   set_ipv6_addr+0x3dd/0x460
   do_execute_actions+0x6a3d/0x7c40
   ovs_execute_actions+0xfd/0x480
   ovs_packet_cmd_execute+0xc38/0xf20
   genl_rcv_msg+0x59e/0x870
   netlink_rcv_skb+0x18b/0x450
   genl_rcv+0x2d/0x40
   netlink_unicast+0x6bc/0xa20

  The buggy address belongs to the object at ffff888131a4c980
   which belongs to the cache skbuff_small_head of size 704
  The buggy address is located 390 bytes inside of
   freed 704-byte region [ffff888131a4c980, ffff888131a4cc40)

ipv6_find_hdr() walks the extension header chain, skipping each header
by the length that header itself declares, and ipv6_optlen() returns up
to 2048.  The last skip is never checked against skb->len, and
parse_ipv6hdr() installs the result as the transport header.

Reject a negative length.

Fixes: ccb1352e76cf ("net: Add Open vSwitch kernel components.")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Norbert Szetei <norbert@doyensec.com>
---
Reproducer available on request.

 net/openvswitch/actions.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index dc5ff859f114..4fdc08cca058 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c
@@ -360,6 +360,9 @@ static void update_ipv6_checksum(struct sk_buff *skb, u8 l4_proto,
 {
 	int transport_len = skb->len - skb_transport_offset(skb);
 
+	if (unlikely(transport_len < 0))
+		return;
+
 	if (l4_proto == NEXTHDR_TCP) {
 		if (likely(transport_len >= sizeof(struct tcphdr)))
 			inet_proto_csum_replace16(&tcp_hdr(skb)->check, skb,
-- 
2.55.0

Re: [PATCH net] openvswitch: reject a negative transport length in the IPv6 L4 checksum update

Posted by Ilya Maximets 1 week, 2 days ago

On 9/15/26 9:55 PM, Norbert Szetei wrote:
> update_ipv6_checksum() computes
> 
> 	int transport_len = skb->len - skb_transport_offset(skb);
> 
> and gates the L4 access on
> 
> 	if (likely(transport_len >= sizeof(struct tcphdr)))
> 
> sizeof yields size_t, so transport_len is converted to unsigned and every
> negative value passes the guard.  inet_proto_csum_replace16() then reads
> the transport checksum field out of bounds, and writes the updated value
> back to the same address:
> 
>   BUG: KASAN: slab-use-after-free in inet_proto_csum_replace16+0x445/0x470
>   Read of size 2 at addr ffff888131a4cb06 by task ovs_ipv6_oob/696
>   CPU: 3 UID: 1000 PID: 696 Comm: ovs_ipv6_oob Tainted: G N 7.3.0-rc2+ #338
>   Call Trace:
>    inet_proto_csum_replace16+0x445/0x470
>    set_ipv6_addr+0x3dd/0x460
>    do_execute_actions+0x6a3d/0x7c40
>    ovs_execute_actions+0xfd/0x480
>    ovs_packet_cmd_execute+0xc38/0xf20
>    genl_rcv_msg+0x59e/0x870
>    netlink_rcv_skb+0x18b/0x450
>    genl_rcv+0x2d/0x40
>    netlink_unicast+0x6bc/0xa20
> 
>   The buggy address belongs to the object at ffff888131a4c980
>    which belongs to the cache skbuff_small_head of size 704
>   The buggy address is located 390 bytes inside of
>    freed 704-byte region [ffff888131a4c980, ffff888131a4cc40)
> 
> ipv6_find_hdr() walks the extension header chain, skipping each header
> by the length that header itself declares, and ipv6_optlen() returns up
> to 2048.  The last skip is never checked against skb->len, and
> parse_ipv6hdr() installs the result as the transport header.

Hi, Norbert.

This looks like something that needs to be fixed in the ipv6_find_hdr().
IMO, this function should not return an offset that doesn't exist in the
packet, i.e. we should not be able to find what's not in there.

CC: David and Ido.

In general, we should be able to trust the skb_transport_offset(),
otherwise there could be other issues all over the place, not only in
openvswitch.

Best regards, Ilya Maximets.

> 
> Reject a negative length.
> 
> Fixes: ccb1352e76cf ("net: Add Open vSwitch kernel components.")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Norbert Szetei <norbert@doyensec.com>
> ---
> Reproducer available on request.
> 
>  net/openvswitch/actions.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
> index dc5ff859f114..4fdc08cca058 100644
> --- a/net/openvswitch/actions.c
> +++ b/net/openvswitch/actions.c
> @@ -360,6 +360,9 @@ static void update_ipv6_checksum(struct sk_buff *skb, u8 l4_proto,
>  {
>  	int transport_len = skb->len - skb_transport_offset(skb);
>  
> +	if (unlikely(transport_len < 0))
> +		return;
> +
>  	if (l4_proto == NEXTHDR_TCP) {
>  		if (likely(transport_len >= sizeof(struct tcphdr)))
>  			inet_proto_csum_replace16(&tcp_hdr(skb)->check, skb,

Re: [PATCH net] openvswitch: reject a negative transport length in the IPv6 L4 checksum update

Posted by Eric Dumazet 1 week, 2 days ago

On Tue, Sep 15, 2026 at 2:13 PM Ilya Maximets <i.maximets@ovn.org> wrote:
>
> On 9/15/26 9:55 PM, Norbert Szetei wrote:
> > update_ipv6_checksum() computes
> >
> >       int transport_len = skb->len - skb_transport_offset(skb);
> >
> > and gates the L4 access on
> >
> >       if (likely(transport_len >= sizeof(struct tcphdr)))
> >
> > sizeof yields size_t, so transport_len is converted to unsigned and every
> > negative value passes the guard.  inet_proto_csum_replace16() then reads
> > the transport checksum field out of bounds, and writes the updated value
> > back to the same address:
> >
> >   BUG: KASAN: slab-use-after-free in inet_proto_csum_replace16+0x445/0x470
> >   Read of size 2 at addr ffff888131a4cb06 by task ovs_ipv6_oob/696
> >   CPU: 3 UID: 1000 PID: 696 Comm: ovs_ipv6_oob Tainted: G N 7.3.0-rc2+ #338
> >   Call Trace:
> >    inet_proto_csum_replace16+0x445/0x470
> >    set_ipv6_addr+0x3dd/0x460
> >    do_execute_actions+0x6a3d/0x7c40
> >    ovs_execute_actions+0xfd/0x480
> >    ovs_packet_cmd_execute+0xc38/0xf20
> >    genl_rcv_msg+0x59e/0x870
> >    netlink_rcv_skb+0x18b/0x450
> >    genl_rcv+0x2d/0x40
> >    netlink_unicast+0x6bc/0xa20
> >
> >   The buggy address belongs to the object at ffff888131a4c980
> >    which belongs to the cache skbuff_small_head of size 704
> >   The buggy address is located 390 bytes inside of
> >    freed 704-byte region [ffff888131a4c980, ffff888131a4cc40)
> >
> > ipv6_find_hdr() walks the extension header chain, skipping each header
> > by the length that header itself declares, and ipv6_optlen() returns up
> > to 2048.  The last skip is never checked against skb->len, and
> > parse_ipv6hdr() installs the result as the transport header.
>
> Hi, Norbert.
>
> This looks like something that needs to be fixed in the ipv6_find_hdr().
> IMO, this function should not return an offset that doesn't exist in the
> packet, i.e. we should not be able to find what's not in there.
>
> CC: David and Ido.
>
> In general, we should be able to trust the skb_transport_offset(),
> otherwise there could be other issues all over the place, not only in
> openvswitch.

Totally agree.
We keep receiving defensive code instead of finding and fixing the
root causes :/

Fix ipv6_find_hdr() in net/ipv6/exthdrs_core.c

if (skb->len - start < hdrlen)
    return -EBADMSG;

Re: [PATCH net] openvswitch: reject a negative transport length in the IPv6 L4 checksum update

Posted by Norbert Szetei 1 week, 1 day ago

On Sep 15, 2026, at 23:40, Eric Dumazet <edumazet@google.com> wrote:
> 
> On Tue, Sep 15, 2026 at 2:13 PM Ilya Maximets <i.maximets@ovn.org> wrote:
>> 
>> 
>> Hi, Norbert.
>> 
>> This looks like something that needs to be fixed in the ipv6_find_hdr().
>> IMO, this function should not return an offset that doesn't exist in the
>> packet, i.e. we should not be able to find what's not in there.
>> 
>> CC: David and Ido.
>> 
>> In general, we should be able to trust the skb_transport_offset(),
>> otherwise there could be other issues all over the place, not only in
>> openvswitch.
> 
> Totally agree.
> We keep receiving defensive code instead of finding and fixing the
> root causes :/
> 
> Fix ipv6_find_hdr() in net/ipv6/exthdrs_core.c
> 
> if (skb->len - start < hdrlen)
>    return -EBADMSG;

Thanks Ilya & Eric, and sorry for the defensive fix. I suspected
ipv6_find_hdr() was the real problem, but I could only reproduce it 
through openvswitch here, so fixing the caller I had evidence for 
seemed like the smaller change. I'll send a v2 for ipv6_find_hdr() 
later.

N.