mbox series

[net-next,v3,0/9] do not leave dangling sk pointers in pf->create functions

Message ID 20241014153808.51894-1-ignat@cloudflare.com
Headers show
Series do not leave dangling sk pointers in pf->create functions | expand

Message

Ignat Korchagin Oct. 14, 2024, 3:37 p.m. UTC
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(-)

Comments

Willem de Bruijn Oct. 15, 2024, 12:52 a.m. UTC | #1
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>
Eric Dumazet Oct. 15, 2024, 8:01 a.m. UTC | #2
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.
patchwork-bot+netdevbpf@kernel.org Oct. 16, 2024, 1:50 a.m. UTC | #3
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!
patchwork-bot+bluetooth@kernel.org Oct. 16, 2024, 7:05 p.m. UTC | #4
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!