net/ipv4/tcp_ao.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
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
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 >
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
© 2016 - 2025 Red Hat, Inc.