[PATCH] net/tcp: Fix a NULL pointer dereference when using TCP-AO with TCP_REPAIR.

Anderson Nascimento posted 1 patch 3 weeks ago
There is a newer version of this series
net/ipv4/tcp_ao.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
[PATCH] net/tcp: Fix a NULL pointer dereference when using TCP-AO with TCP_REPAIR.
Posted by Anderson Nascimento 3 weeks ago
A NULL pointer dereference can occur in tcp_ao_finish_connect() during a connect() system call on a socket with a TCP-AO key added and TCP_REPAIR enabled.

The function is called with skb being NULL and attempts to dereference it on tcp_hdr(skb)->seq without a prior skb validation.

Fix this by checking if skb is NULL before dereferencing it. If skb is not NULL, the ao->risn is set to tcp_hdr(skb)->seq to keep compatibility with the call made from tcp_rcv_synsent_state_process(). If skb is NULL, ao->risn is set to 0.

The commentary is taken from bpf_skops_established(), which is also called in the same flow. Unlike the function being patched, bpf_skops_established() validates the skb before dereferencing it.

int main(void){
        struct sockaddr_in sockaddr;
        struct tcp_ao_add tcp_ao;
        int sk;
        int one = 1;

        memset(&sockaddr,'\0',sizeof(sockaddr));
        memset(&tcp_ao,'\0',sizeof(tcp_ao));

        sk = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);

        sockaddr.sin_family = AF_INET;

        memcpy(tcp_ao.alg_name,"cmac(aes128)",12);
        memcpy(tcp_ao.key,"ABCDEFGHABCDEFGH",16);
        tcp_ao.keylen = 16;

        memcpy(&tcp_ao.addr,&sockaddr,sizeof(sockaddr));

        setsockopt(sk, IPPROTO_TCP, TCP_AO_ADD_KEY, &tcp_ao, sizeof(tcp_ao));
        setsockopt(sk, IPPROTO_TCP, TCP_REPAIR, &one, sizeof(one));

        sockaddr.sin_family = AF_INET;
        sockaddr.sin_port = htobe16(123);

        inet_aton("127.0.0.1", &sockaddr.sin_addr);

        connect(sk,(struct sockaddr *)&sockaddr,sizeof(sockaddr));

return 0;
}

$ gcc tcp-ao-nullptr.c -o tcp-ao-nullptr -Wall
$ unshare -Urn
# ip addr add 127.0.0.1 dev lo
# ./tcp-ao-nullptr

[   72.414850] BUG: kernel NULL pointer dereference, address: 00000000000000b6
[   72.414863] #PF: supervisor read access in kernel mode
[   72.414869] #PF: error_code(0x0000) - not-present page
[   72.414873] PGD 116af4067 P4D 116af4067 PUD 117043067 PMD 0
[   72.414880] Oops: Oops: 0000 [#1] SMP NOPTI
[   72.414887] CPU: 2 UID: 1000 PID: 1558 Comm: tcp-ao-nullptr Not tainted 6.16.3-200.fc42.x86_64 #1 PREEMPT(lazy)
[   72.414896] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 11/12/2020
[   72.414905] RIP: 0010:tcp_ao_finish_connect+0x19/0x60

Fixes: 7c2ffaf ("net/tcp: Calculate TCP-AO traffic keys")
Signed-off-by: Anderson Nascimento <anderson@allelesecurity.com>
---
 net/ipv4/tcp_ao.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/net/ipv4/tcp_ao.c b/net/ipv4/tcp_ao.c
index bbb8d5f0eae7..abe913de8652 100644
--- a/net/ipv4/tcp_ao.c
+++ b/net/ipv4/tcp_ao.c
@@ -1178,7 +1178,11 @@ void tcp_ao_finish_connect(struct sock *sk, struct sk_buff *skb)
 	if (!ao)
 		return;
 
-	WRITE_ONCE(ao->risn, tcp_hdr(skb)->seq);
+	/* sk with TCP_REPAIR_ON does not have skb in tcp_finish_connect */
+	if (skb)
+		WRITE_ONCE(ao->risn, tcp_hdr(skb)->seq);
+	else
+		WRITE_ONCE(ao->risn, 0);
 	ao->rcv_sne = 0;
 
 	hlist_for_each_entry_rcu(key, &ao->head, node, lockdep_sock_is_held(sk))
-- 
2.51.0
Re: [PATCH] net/tcp: Fix a NULL pointer dereference when using TCP-AO with TCP_REPAIR.
Posted by Kuniyuki Iwashima 3 weeks ago
On Wed, Sep 10, 2025 at 6:32 PM Anderson Nascimento
<anderson@allelesecurity.com> wrote:
>
> A NULL pointer dereference can occur in tcp_ao_finish_connect() during a connect() system call on a socket with a TCP-AO key added and TCP_REPAIR enabled.

Thanks for the patch, the change looks good.

Could you wrap the description at 75 columns ?

See this doc for other guidelines:
https://www.kernel.org/doc/html/latest/process/submitting-patches.html#the-canonical-patch-format


>
> The function is called with skb being NULL and attempts to dereference it on tcp_hdr(skb)->seq without a prior skb validation.
>
> Fix this by checking if skb is NULL before dereferencing it. If skb is not NULL, the ao->risn is set to tcp_hdr(skb)->seq to keep compatibility with the call made from tcp_rcv_synsent_state_process(). If skb is NULL, ao->risn is set to 0.
>
> The commentary is taken from bpf_skops_established(), which is also called in the same flow. Unlike the function being patched, bpf_skops_established() validates the skb before dereferencing it.
>
> int main(void){
>         struct sockaddr_in sockaddr;
>         struct tcp_ao_add tcp_ao;
>         int sk;
>         int one = 1;
>
>         memset(&sockaddr,'\0',sizeof(sockaddr));
>         memset(&tcp_ao,'\0',sizeof(tcp_ao));
>
>         sk = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
>
>         sockaddr.sin_family = AF_INET;
>
>         memcpy(tcp_ao.alg_name,"cmac(aes128)",12);
>         memcpy(tcp_ao.key,"ABCDEFGHABCDEFGH",16);
>         tcp_ao.keylen = 16;
>
>         memcpy(&tcp_ao.addr,&sockaddr,sizeof(sockaddr));
>
>         setsockopt(sk, IPPROTO_TCP, TCP_AO_ADD_KEY, &tcp_ao, sizeof(tcp_ao));
>         setsockopt(sk, IPPROTO_TCP, TCP_REPAIR, &one, sizeof(one));
>
>         sockaddr.sin_family = AF_INET;
>         sockaddr.sin_port = htobe16(123);
>
>         inet_aton("127.0.0.1", &sockaddr.sin_addr);
>
>         connect(sk,(struct sockaddr *)&sockaddr,sizeof(sockaddr));
>
> return 0;
> }
>
> $ gcc tcp-ao-nullptr.c -o tcp-ao-nullptr -Wall
> $ unshare -Urn
> # ip addr add 127.0.0.1 dev lo
> # ./tcp-ao-nullptr
>
> [   72.414850] BUG: kernel NULL pointer dereference, address: 00000000000000b6
> [   72.414863] #PF: supervisor read access in kernel mode
> [   72.414869] #PF: error_code(0x0000) - not-present page
> [   72.414873] PGD 116af4067 P4D 116af4067 PUD 117043067 PMD 0
> [   72.414880] Oops: Oops: 0000 [#1] SMP NOPTI
> [   72.414887] CPU: 2 UID: 1000 PID: 1558 Comm: tcp-ao-nullptr Not tainted 6.16.3-200.fc42.x86_64 #1 PREEMPT(lazy)
> [   72.414896] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 11/12/2020
> [   72.414905] RIP: 0010:tcp_ao_finish_connect+0x19/0x60

Full decoded stack trace without timestamps would be nicer.

How to decode stack trace:
cat trace.txt | ./scripts/decode_stacktrace.sh vmlinux

>
> Fixes: 7c2ffaf ("net/tcp: Calculate TCP-AO traffic keys")
> Signed-off-by: Anderson Nascimento <anderson@allelesecurity.com>
> ---
>  net/ipv4/tcp_ao.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/net/ipv4/tcp_ao.c b/net/ipv4/tcp_ao.c
> index bbb8d5f0eae7..abe913de8652 100644
> --- a/net/ipv4/tcp_ao.c
> +++ b/net/ipv4/tcp_ao.c
> @@ -1178,7 +1178,11 @@ void tcp_ao_finish_connect(struct sock *sk, struct sk_buff *skb)
>         if (!ao)
>                 return;
>
> -       WRITE_ONCE(ao->risn, tcp_hdr(skb)->seq);
> +       /* sk with TCP_REPAIR_ON does not have skb in tcp_finish_connect */
> +       if (skb)
> +               WRITE_ONCE(ao->risn, tcp_hdr(skb)->seq);
> +       else
> +               WRITE_ONCE(ao->risn, 0);
>         ao->rcv_sne = 0;
>
>         hlist_for_each_entry_rcu(key, &ao->head, node, lockdep_sock_is_held(sk))
> --
> 2.51.0
>
Re: [PATCH] net/tcp: Fix a NULL pointer dereference when using TCP-AO with TCP_REPAIR.
Posted by Anderson Nascimento 3 weeks ago
On Wed, Sep 10, 2025 at 11:44 PM Kuniyuki Iwashima <kuniyu@google.com> wrote:
>
> On Wed, Sep 10, 2025 at 6:32 PM Anderson Nascimento
> <anderson@allelesecurity.com> wrote:
> >
> > A NULL pointer dereference can occur in tcp_ao_finish_connect() during a connect() system call on a socket with a TCP-AO key added and TCP_REPAIR enabled.
>
> Thanks for the patch, the change looks good.
>
> Could you wrap the description at 75 columns ?
>
> See this doc for other guidelines:
> https://www.kernel.org/doc/html/latest/process/submitting-patches.html#the-canonical-patch-format
>
>

Thank you, Kuniyuki. I have just sent a v2 with the changes you
suggested. I hope it's fine now.

> >
> > The function is called with skb being NULL and attempts to dereference it on tcp_hdr(skb)->seq without a prior skb validation.
> >
> > Fix this by checking if skb is NULL before dereferencing it. If skb is not NULL, the ao->risn is set to tcp_hdr(skb)->seq to keep compatibility with the call made from tcp_rcv_synsent_state_process(). If skb is NULL, ao->risn is set to 0.
> >
> > The commentary is taken from bpf_skops_established(), which is also called in the same flow. Unlike the function being patched, bpf_skops_established() validates the skb before dereferencing it.
> >
> > int main(void){
> >         struct sockaddr_in sockaddr;
> >         struct tcp_ao_add tcp_ao;
> >         int sk;
> >         int one = 1;
> >
> >         memset(&sockaddr,'\0',sizeof(sockaddr));
> >         memset(&tcp_ao,'\0',sizeof(tcp_ao));
> >
> >         sk = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
> >
> >         sockaddr.sin_family = AF_INET;
> >
> >         memcpy(tcp_ao.alg_name,"cmac(aes128)",12);
> >         memcpy(tcp_ao.key,"ABCDEFGHABCDEFGH",16);
> >         tcp_ao.keylen = 16;
> >
> >         memcpy(&tcp_ao.addr,&sockaddr,sizeof(sockaddr));
> >
> >         setsockopt(sk, IPPROTO_TCP, TCP_AO_ADD_KEY, &tcp_ao, sizeof(tcp_ao));
> >         setsockopt(sk, IPPROTO_TCP, TCP_REPAIR, &one, sizeof(one));
> >
> >         sockaddr.sin_family = AF_INET;
> >         sockaddr.sin_port = htobe16(123);
> >
> >         inet_aton("127.0.0.1", &sockaddr.sin_addr);
> >
> >         connect(sk,(struct sockaddr *)&sockaddr,sizeof(sockaddr));
> >
> > return 0;
> > }
> >
> > $ gcc tcp-ao-nullptr.c -o tcp-ao-nullptr -Wall
> > $ unshare -Urn
> > # ip addr add 127.0.0.1 dev lo
> > # ./tcp-ao-nullptr
> >
> > [   72.414850] BUG: kernel NULL pointer dereference, address: 00000000000000b6
> > [   72.414863] #PF: supervisor read access in kernel mode
> > [   72.414869] #PF: error_code(0x0000) - not-present page
> > [   72.414873] PGD 116af4067 P4D 116af4067 PUD 117043067 PMD 0
> > [   72.414880] Oops: Oops: 0000 [#1] SMP NOPTI
> > [   72.414887] CPU: 2 UID: 1000 PID: 1558 Comm: tcp-ao-nullptr Not tainted 6.16.3-200.fc42.x86_64 #1 PREEMPT(lazy)
> > [   72.414896] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 11/12/2020
> > [   72.414905] RIP: 0010:tcp_ao_finish_connect+0x19/0x60
>
> Full decoded stack trace without timestamps would be nicer.
>
> How to decode stack trace:
> cat trace.txt | ./scripts/decode_stacktrace.sh vmlinux
>
> >
> > Fixes: 7c2ffaf ("net/tcp: Calculate TCP-AO traffic keys")
> > Signed-off-by: Anderson Nascimento <anderson@allelesecurity.com>
> > ---
> >  net/ipv4/tcp_ao.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/net/ipv4/tcp_ao.c b/net/ipv4/tcp_ao.c
> > index bbb8d5f0eae7..abe913de8652 100644
> > --- a/net/ipv4/tcp_ao.c
> > +++ b/net/ipv4/tcp_ao.c
> > @@ -1178,7 +1178,11 @@ void tcp_ao_finish_connect(struct sock *sk, struct sk_buff *skb)
> >         if (!ao)
> >                 return;
> >
> > -       WRITE_ONCE(ao->risn, tcp_hdr(skb)->seq);
> > +       /* sk with TCP_REPAIR_ON does not have skb in tcp_finish_connect */
> > +       if (skb)
> > +               WRITE_ONCE(ao->risn, tcp_hdr(skb)->seq);
> > +       else
> > +               WRITE_ONCE(ao->risn, 0);
> >         ao->rcv_sne = 0;
> >
> >         hlist_for_each_entry_rcu(key, &ao->head, node, lockdep_sock_is_held(sk))
> > --
> > 2.51.0
> >



-- 
Anderson Nascimento
Allele Security Intelligence
https://www.allelesecurity.com