| Message ID | 20241014153808.51894-2-ignat@cloudflare.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| 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:00 +0100 > After sock_init_data() the allocated sk object is attached to the provided > sock object. On error, packet_create() frees the sk object leaving the > dangling pointer in the sock object on return. Some other code may try > to use this pointer and cause use-after-free. > > Suggested-by: Eric Dumazet <edumazet@google.com> > Signed-off-by: Ignat Korchagin <ignat@cloudflare.com> Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com>
Ignat Korchagin wrote: > After sock_init_data() the allocated sk object is attached to the provided > sock object. On error, packet_create() frees the sk object leaving the > dangling pointer in the sock object on return. Some other code may try > to use this pointer and cause use-after-free. > > Suggested-by: Eric Dumazet <edumazet@google.com> > Signed-off-by: Ignat Korchagin <ignat@cloudflare.com> Reviewed-by: Willem de Bruijn <willemb@google.com>
On Mon, Oct 14, 2024 at 5:38 PM Ignat Korchagin <ignat@cloudflare.com> wrote: > > After sock_init_data() the allocated sk object is attached to the provided > sock object. On error, packet_create() frees the sk object leaving the > dangling pointer in the sock object on return. Some other code may try > to use this pointer and cause use-after-free. > > Suggested-by: Eric Dumazet <edumazet@google.com> > Signed-off-by: Ignat Korchagin <ignat@cloudflare.com> > --- Reviewed-by: Eric Dumazet <edumazet@google.com> Thanks.
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c index f8942062f776..99ae27d1e4dc 100644 --- a/net/packet/af_packet.c +++ b/net/packet/af_packet.c @@ -3421,17 +3421,17 @@ static int packet_create(struct net *net, struct socket *sock, int protocol, if (sock->type == SOCK_PACKET) sock->ops = &packet_ops_spkt; + po = pkt_sk(sk); + err = packet_alloc_pending(po); + if (err) + goto out_sk_free; + sock_init_data(sock, sk); - po = pkt_sk(sk); init_completion(&po->skb_completion); sk->sk_family = PF_PACKET; po->num = proto; - err = packet_alloc_pending(po); - if (err) - goto out2; - packet_cached_dev_reset(po); sk->sk_destruct = packet_sock_destruct; @@ -3463,7 +3463,7 @@ static int packet_create(struct net *net, struct socket *sock, int protocol, sock_prot_inuse_add(net, &packet_proto, 1); return 0; -out2: +out_sk_free: sk_free(sk); out: return err;
After sock_init_data() the allocated sk object is attached to the provided sock object. On error, packet_create() frees the sk object leaving the dangling pointer in the sock object on return. Some other code may try to use this pointer and cause use-after-free. Suggested-by: Eric Dumazet <edumazet@google.com> Signed-off-by: Ignat Korchagin <ignat@cloudflare.com> --- net/packet/af_packet.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)