| Message ID | 20241014153808.51894-1-ignat@cloudflare.com |
|---|---|
| Headers | show |
| Series | do not leave dangling sk pointers in pf->create functions | expand |
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.
Hello: This series was applied to netdev/net-next.git (main) by Jakub Kicinski <kuba@kernel.org>: On Mon, 14 Oct 2024 16:37:59 +0100 you wrote: > Some protocol family create() implementations have an error path after > allocating the sk object and calling sock_init_data(). sock_init_data() > attaches the allocated sk object to the sock object, provided by the > caller. > > If the create() implementation errors out after calling sock_init_data(), > it releases the allocated sk object, but the caller ends up having a > dangling sk pointer in its sock object on return. Subsequent manipulations > on this sock object may try to access the sk pointer, because it is not > NULL thus creating a use-after-free scenario. > > [...] Here is the summary with links: - [net-next,v3,1/9] af_packet: avoid erroring out after sock_init_data() in packet_create() https://git.kernel.org/netdev/net-next/c/46f2a11cb82b - [net-next,v3,2/9] Bluetooth: L2CAP: do not leave dangling sk pointer on error in l2cap_sock_create() https://git.kernel.org/netdev/net-next/c/7c4f78cdb8e7 - [net-next,v3,3/9] Bluetooth: RFCOMM: avoid leaving dangling sk pointer in rfcomm_sock_alloc() https://git.kernel.org/netdev/net-next/c/3945c799f12b - [net-next,v3,4/9] net: af_can: do not leave a dangling sk pointer in can_create() https://git.kernel.org/netdev/net-next/c/811a7ca7320c - [net-next,v3,5/9] net: ieee802154: do not leave a dangling sk pointer in ieee802154_create() https://git.kernel.org/netdev/net-next/c/b4fcd63f6ef7 - [net-next,v3,6/9] net: inet: do not leave a dangling sk pointer in inet_create() https://git.kernel.org/netdev/net-next/c/9365fa510c6f - [net-next,v3,7/9] net: inet6: do not leave a dangling sk pointer in inet6_create() https://git.kernel.org/netdev/net-next/c/9df99c395d0f - [net-next,v3,8/9] net: warn, if pf->create does not clear sock->sk on error https://git.kernel.org/netdev/net-next/c/48156296a08c - [net-next,v3,9/9] Revert "net: do not leave a dangling sk pointer, when socket creation fails" https://git.kernel.org/netdev/net-next/c/18429e6e0c2a You are awesome, thank you!
Hello: This series was applied to bluetooth/bluetooth-next.git (master) by Jakub Kicinski <kuba@kernel.org>: On Mon, 14 Oct 2024 16:37:59 +0100 you wrote: > Some protocol family create() implementations have an error path after > allocating the sk object and calling sock_init_data(). sock_init_data() > attaches the allocated sk object to the sock object, provided by the > caller. > > If the create() implementation errors out after calling sock_init_data(), > it releases the allocated sk object, but the caller ends up having a > dangling sk pointer in its sock object on return. Subsequent manipulations > on this sock object may try to access the sk pointer, because it is not > NULL thus creating a use-after-free scenario. > > [...] Here is the summary with links: - [net-next,v3,1/9] af_packet: avoid erroring out after sock_init_data() in packet_create() https://git.kernel.org/bluetooth/bluetooth-next/c/46f2a11cb82b - [net-next,v3,2/9] Bluetooth: L2CAP: do not leave dangling sk pointer on error in l2cap_sock_create() https://git.kernel.org/bluetooth/bluetooth-next/c/7c4f78cdb8e7 - [net-next,v3,3/9] Bluetooth: RFCOMM: avoid leaving dangling sk pointer in rfcomm_sock_alloc() https://git.kernel.org/bluetooth/bluetooth-next/c/3945c799f12b - [net-next,v3,4/9] net: af_can: do not leave a dangling sk pointer in can_create() https://git.kernel.org/bluetooth/bluetooth-next/c/811a7ca7320c - [net-next,v3,5/9] net: ieee802154: do not leave a dangling sk pointer in ieee802154_create() https://git.kernel.org/bluetooth/bluetooth-next/c/b4fcd63f6ef7 - [net-next,v3,6/9] net: inet: do not leave a dangling sk pointer in inet_create() https://git.kernel.org/bluetooth/bluetooth-next/c/9365fa510c6f - [net-next,v3,7/9] net: inet6: do not leave a dangling sk pointer in inet6_create() https://git.kernel.org/bluetooth/bluetooth-next/c/9df99c395d0f - [net-next,v3,8/9] net: warn, if pf->create does not clear sock->sk on error https://git.kernel.org/bluetooth/bluetooth-next/c/48156296a08c - [net-next,v3,9/9] Revert "net: do not leave a dangling sk pointer, when socket creation fails" https://git.kernel.org/bluetooth/bluetooth-next/c/18429e6e0c2a You are awesome, thank you!
Some protocol family create() implementations have an error path after allocating the sk object and calling sock_init_data(). sock_init_data() attaches the allocated sk object to the sock object, provided by the caller. If the create() implementation errors out after calling sock_init_data(), it releases the allocated sk object, but the caller ends up having a dangling sk pointer in its sock object on return. Subsequent manipulations on this sock object may try to access the sk pointer, because it is not NULL thus creating a use-after-free scenario. We have implemented a stable hotfix in commit 631083143315 ("net: explicitly clear the sk pointer, when pf->create fails"), but this series aims to fix it properly by going through each of the pf->create() implementations and making sure they all don't return a sock object with a dangling pointer on error. Changes in V3: * retargeted the series to net-next * dropped the hotfix patch, which was merged into net already * replaced the hotfix code with DEBUG_NET_WARN_ON_ONCE() to catch future violations Changes in V2: * reverted the change introduced in 6cd4a78d962b ("net: do not leave a dangling sk pointer, when socket creation fails") * added optional commits to all pf->create implementaions to clear the sk pointer on error after sock_init_data() Ignat Korchagin (9): af_packet: avoid erroring out after sock_init_data() in packet_create() Bluetooth: L2CAP: do not leave dangling sk pointer on error in l2cap_sock_create() Bluetooth: RFCOMM: avoid leaving dangling sk pointer in rfcomm_sock_alloc() net: af_can: do not leave a dangling sk pointer in can_create() net: ieee802154: do not leave a dangling sk pointer in ieee802154_create() net: inet: do not leave a dangling sk pointer in inet_create() net: inet6: do not leave a dangling sk pointer in inet6_create() net: warn, if pf->create does not clear sock->sk on error Revert "net: do not leave a dangling sk pointer, when socket creation fails" net/bluetooth/l2cap_sock.c | 1 + net/bluetooth/rfcomm/sock.c | 10 +++++----- net/can/af_can.c | 1 + net/core/sock.c | 3 --- net/ieee802154/socket.c | 12 +++++++----- net/ipv4/af_inet.c | 22 ++++++++++------------ net/ipv6/af_inet6.c | 22 ++++++++++------------ net/packet/af_packet.c | 12 ++++++------ net/socket.c | 4 ++-- 9 files changed, 42 insertions(+), 45 deletions(-)