| Message ID | 20241014153808.51894-9-ignat@cloudflare.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 48156296a08c615a6baae514096c4b2e543d1157 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | do not leave dangling sk pointers in pf->create functions | expand |
From: Ignat Korchagin <ignat@cloudflare.com> Date: Mon, 14 Oct 2024 16:38:07 +0100 > All pf->create implementations have been fixed now to clear sock->sk on > error, when they deallocate the allocated sk object. > > Put a warning in place to make sure we don't break this promise in the > future. > > Suggested-by: Kuniyuki Iwashima <kuniyu@amazon.com> > Signed-off-by: Ignat Korchagin <ignat@cloudflare.com> Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com>
On Mon, Oct 14, 2024 at 11:40 PM Kuniyuki Iwashima <kuniyu@amazon.com> wrote: > > From: Ignat Korchagin <ignat@cloudflare.com> > Date: Mon, 14 Oct 2024 16:38:07 +0100 > > All pf->create implementations have been fixed now to clear sock->sk on > > error, when they deallocate the allocated sk object. > > > > Put a warning in place to make sure we don't break this promise in the > > future. > > > > Suggested-by: Kuniyuki Iwashima <kuniyu@amazon.com> > > Signed-off-by: Ignat Korchagin <ignat@cloudflare.com> > > Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com> Reviewed-by: Eric Dumazet <edumazet@google.com>
diff --git a/net/socket.c b/net/socket.c index 24b404299015..9a8e4452b9b2 100644 --- a/net/socket.c +++ b/net/socket.c @@ -1576,9 +1576,9 @@ int __sock_create(struct net *net, int family, int type, int protocol, err = pf->create(net, sock, protocol, kern); if (err < 0) { /* ->create should release the allocated sock->sk object on error - * but it may leave the dangling pointer + * and make sure sock->sk is set to NULL to avoid use-after-free */ - sock->sk = NULL; + DEBUG_NET_WARN_ON_ONCE(sock->sk); goto out_module_put; }
All pf->create implementations have been fixed now to clear sock->sk on error, when they deallocate the allocated sk object. Put a warning in place to make sure we don't break this promise in the future. Suggested-by: Kuniyuki Iwashima <kuniyu@amazon.com> Signed-off-by: Ignat Korchagin <ignat@cloudflare.com> --- net/socket.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)