| Message ID | 20210504071646.28665-1-orcohen@paloaltonetworks.com |
|---|---|
| State | New |
| Headers | show |
| Series | net/nfc: fix use-after-free llcp_sock_bind/connect | expand |
On Tue, May 04, 2021 at 07:10:10PM +0000, patchwork-bot+netdevbpf@kernel.org wrote: > Hello: > > This patch was applied to netdev/net.git (refs/heads/master): > > On Tue, 4 May 2021 10:16:46 +0300 you wrote: > > Commits 8a4cd82d ("nfc: fix refcount leak in llcp_sock_connect()") > > and c33b1cc62 ("nfc: fix refcount leak in llcp_sock_bind()") > > fixed a refcount leak bug in bind/connect but introduced a > > use-after-free if the same local is assigned to 2 different sockets. > > > > This can be triggered by the following simple program: > > int sock1 = socket( AF_NFC, SOCK_STREAM, NFC_SOCKPROTO_LLCP ); > > int sock2 = socket( AF_NFC, SOCK_STREAM, NFC_SOCKPROTO_LLCP ); > > memset( &addr, 0, sizeof(struct sockaddr_nfc_llcp) ); > > addr.sa_family = AF_NFC; > > addr.nfc_protocol = NFC_PROTO_NFC_DEP; > > bind( sock1, (struct sockaddr*) &addr, sizeof(struct sockaddr_nfc_llcp) ) > > bind( sock2, (struct sockaddr*) &addr, sizeof(struct sockaddr_nfc_llcp) ) > > close(sock1); > > close(sock2); > > > > [...] > > Here is the summary with links: > - net/nfc: fix use-after-free llcp_sock_bind/connect > https://git.kernel.org/netdev/net/c/c61760e6940d Dave, Can you please share your thoughts how this patch can be correct? https://lore.kernel.org/netdev/YJIjN6MTRdQ7Bvcp@unreal/T/#m1e67ae6c2658312a134f65819c5ad92511f207c1 It is also under review, so unclear why it was merged. Thanks > > You are awesome, thank you! > -- > Deet-doot-dot, I am a bot. > https://korg.docs.kernel.org/patchwork/pwbot.html > >
diff --git a/net/nfc/llcp_sock.c b/net/nfc/llcp_sock.c index a3b46f888803..53dbe733f998 100644 --- a/net/nfc/llcp_sock.c +++ b/net/nfc/llcp_sock.c @@ -109,12 +109,14 @@ static int llcp_sock_bind(struct socket *sock, struct sockaddr *addr, int alen) GFP_KERNEL); if (!llcp_sock->service_name) { nfc_llcp_local_put(llcp_sock->local); + llcp_sock->local = NULL; ret = -ENOMEM; goto put_dev; } llcp_sock->ssap = nfc_llcp_get_sdp_ssap(local, llcp_sock); if (llcp_sock->ssap == LLCP_SAP_MAX) { nfc_llcp_local_put(llcp_sock->local); + llcp_sock->local = NULL; kfree(llcp_sock->service_name); llcp_sock->service_name = NULL; ret = -EADDRINUSE; @@ -709,6 +711,7 @@ static int llcp_sock_connect(struct socket *sock, struct sockaddr *_addr, llcp_sock->ssap = nfc_llcp_get_local_ssap(local); if (llcp_sock->ssap == LLCP_SAP_MAX) { nfc_llcp_local_put(llcp_sock->local); + llcp_sock->local = NULL; ret = -ENOMEM; goto put_dev; } @@ -756,6 +759,7 @@ static int llcp_sock_connect(struct socket *sock, struct sockaddr *_addr, sock_llcp_release: nfc_llcp_put_ssap(local, llcp_sock->ssap); nfc_llcp_local_put(llcp_sock->local); + llcp_sock->local = NULL; put_dev: nfc_put_device(dev);
Commits 8a4cd82d ("nfc: fix refcount leak in llcp_sock_connect()") and c33b1cc62 ("nfc: fix refcount leak in llcp_sock_bind()") fixed a refcount leak bug in bind/connect but introduced a use-after-free if the same local is assigned to 2 different sockets. This can be triggered by the following simple program: int sock1 = socket( AF_NFC, SOCK_STREAM, NFC_SOCKPROTO_LLCP ); int sock2 = socket( AF_NFC, SOCK_STREAM, NFC_SOCKPROTO_LLCP ); memset( &addr, 0, sizeof(struct sockaddr_nfc_llcp) ); addr.sa_family = AF_NFC; addr.nfc_protocol = NFC_PROTO_NFC_DEP; bind( sock1, (struct sockaddr*) &addr, sizeof(struct sockaddr_nfc_llcp) ) bind( sock2, (struct sockaddr*) &addr, sizeof(struct sockaddr_nfc_llcp) ) close(sock1); close(sock2); Fix this by assigning NULL to llcp_sock->local after calling nfc_llcp_local_put. This addresses CVE-2021-23134. Reported-by: Or Cohen <orcohen@paloaltonetworks.com> Reported-by: Nadav Markus <nmarkus@paloaltonetworks.com> Fixes: c33b1cc62 ("nfc: fix refcount leak in llcp_sock_bind()") Signed-off-by: Or Cohen <orcohen@paloaltonetworks.com> --- net/nfc/llcp_sock.c | 4 ++++ 1 file changed, 4 insertions(+)