GPF in net sybsystem
Subsystems (2)
Hi, netdev developers!
I've spent some time debugging this bug
https://syzkaller.appspot.com/bug?id=c670fb9da2ce08f7b5101baa9426083b39ee9f90
and, I believe, I found the root case:
static int nr_accept(struct socket *sock, struct socket *newsock, int flags,
bool kern)
{
....
for (;;) {
prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE);
...
if (!signal_pending(current)) {
release_sock(sk);
schedule();
lock_sock(sk);
continue;
}
...
}...
}
When calling process will be scheduled, another proccess can release this socket and set sk->sk_wq to NULL. (In this case nr_release() will call sock_orphan(sk)). In this case GPF will happen in prepare_to_wait().
I came up with this patch, but im not an expect in netdev sybsystem and im not sure about this one:
net/netrom/af_netrom.c +4 -0
@@ -803,6 +803,10 @@ static int nr_accept(struct socket *sock, struct socket *newsock, int flags,release_sock (sk);schedule ();lock_sock (sk); +if (sock_flag (sk,SOCK_DEAD )) { + err = -ECONNABORTED ; + goto out_release; + }continue ; } err = -ERESTARTSYS ;
I look forward to hearing your perspective on this :)
BTW, I found similar code in:
1) net/ax25/af_ax25.c
2) net/rose/af_rose.c
I hope, this will help!
With regards,
Pavel Skripkin
Jakub Kicinski
maintainerQuoted 21 lines · On Wed, 5 May 2021 20:02:42 +0300 Pavel Skripkin wrote:
Hi, netdev developers!
I've spent some time debugging this bug
https://syzkaller.appspot.com/bug?id=c670fb9da2ce08f7b5101baa9426083b39ee9f90
and, I believe, I found the root case:static int nr_accept(struct socket *sock, struct socket *newsock, int flags,
bool kern)
{
....for (;;) { prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE); ... if (!signal_pending(current)) { release_sock(sk); schedule(); lock_sock(sk); continue; } ... }
Quoted 7 lines
...
}When calling process will be scheduled, another proccess can release this socket and set
sk->sk_wqto NULL. (In this casenr_release()will callsock_orphan(sk)). In this case GPF will happen inprepare_to_wait().
How does it get released midway through an accept call? Is there a reference imbalance somewhere else in the code?
Quoted 32 lines
I came up with this patch, but im not an expect in netdev sybsystem and im not sure about this one:
net/netrom/af_netrom.c +4
-0@@ -803,6 +803,10 @@ static int nr_accept(struct socket *sock, struct socket *newsock, int flags,release_sock (sk);schedule ();lock_sock (sk); +if (sock_flag (sk,SOCK_DEAD )) { + err = -ECONNABORTED ; + goto out_release; + }continue ; } err = -ERESTARTSYS ;I look forward to hearing your perspective on this :)
BTW, I found similar code in:
1)
net/ax25/af_ax25.c
2)net/rose/af_rose.cI hope, this will help!
With regards,
Pavel Skripkin
Quoted 22 lines · Pavel Skripkin
Hi, netdev developers!
I've spent some time debugging this bug
https://syzkaller.appspot.com/bug?id=c670fb9da2ce08f7b5101baa9426083b39ee9f90
and, I believe, I found the root case:static int nr_accept(struct socket *sock, struct socket *newsock, int flags,
bool kern)
{
....for (;;) { prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE); ... if (!signal_pending(current)) { release_sock(sk); schedule(); lock_sock(sk); continue; } ... }
Quoted 7 lines
...
}When calling process will be scheduled, another proccess can release this socket and set
sk->sk_wqto NULL. (In this casenr_release()will callsock_orphan(sk)). In this case GPF will happen inprepare_to_wait().
Are you sure?
How could another process release this socket when its fd is still refcnt'ed? That is, accept() still does not return yet at the point of schedule().
Also, the above pattern is pretty common in networking subsystem, see sk_wait_event(), so how come it is only problematic for netrom?
Thanks.