* [PATCH v2 1/1] nfc: llcp: fix NULL error pointer dereference on sendmsg() after failed bind()
2022-01-19 7:48 [PATCH v2 0/1] nfc: llcp: a fix after syzbot report Krzysztof Kozlowski
@ 2022-01-19 7:48 ` Krzysztof Kozlowski
2022-01-19 14:20 ` [PATCH v2 0/1] nfc: llcp: a fix after syzbot report patchwork-bot+netdevbpf
1 sibling, 0 replies; 3+ messages in thread
From: Krzysztof Kozlowski @ 2022-01-19 7:48 UTC (permalink / raw)
To: Krzysztof Kozlowski, David S. Miller, Jakub Kicinski, linux-nfc,
netdev, linux-kernel
Cc: syzbot+7f23bcddf626e0593a39, stable
Syzbot detected a NULL pointer dereference of nfc_llcp_sock->dev pointer
(which is a 'struct nfc_dev *') with calls to llcp_sock_sendmsg() after
a failed llcp_sock_bind(). The message being sent is a SOCK_DGRAM.
KASAN report:
BUG: KASAN: null-ptr-deref in nfc_alloc_send_skb+0x2d/0xc0
Read of size 4 at addr 00000000000005c8 by task llcp_sock_nfc_a/899
CPU: 5 PID: 899 Comm: llcp_sock_nfc_a Not tainted 5.16.0-rc6-next-20211224-00001-gc6437fbf18b0 #125
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.14.0-2 04/01/2014
Call Trace:
<TASK>
dump_stack_lvl+0x45/0x59
? nfc_alloc_send_skb+0x2d/0xc0
__kasan_report.cold+0x117/0x11c
? mark_lock+0x480/0x4f0
? nfc_alloc_send_skb+0x2d/0xc0
kasan_report+0x38/0x50
nfc_alloc_send_skb+0x2d/0xc0
nfc_llcp_send_ui_frame+0x18c/0x2a0
? nfc_llcp_send_i_frame+0x230/0x230
? __local_bh_enable_ip+0x86/0xe0
? llcp_sock_connect+0x470/0x470
? llcp_sock_connect+0x470/0x470
sock_sendmsg+0x8e/0xa0
____sys_sendmsg+0x253/0x3f0
...
The issue was visible only with multiple simultaneous calls to bind() and
sendmsg(), which resulted in most of the bind() calls to fail. The
bind() was failing on checking if there is available WKS/SDP/SAP
(respective bit in 'struct nfc_llcp_local' fields). When there was no
available WKS/SDP/SAP, the bind returned error but the sendmsg() to such
socket was able to trigger mentioned NULL pointer dereference of
nfc_llcp_sock->dev.
The code looks simply racy and currently it protects several paths
against race with checks for (!nfc_llcp_sock->local) which is NULL-ified
in error paths of bind(). The llcp_sock_sendmsg() did not have such
check but called function nfc_llcp_send_ui_frame() had, although not
protected with lock_sock().
Therefore the race could look like (same socket is used all the time):
CPU0 CPU1
==== ====
llcp_sock_bind()
- lock_sock()
- success
- release_sock()
- return 0
llcp_sock_sendmsg()
- lock_sock()
- release_sock()
llcp_sock_bind(), same socket
- lock_sock()
- error
- nfc_llcp_send_ui_frame()
- if (!llcp_sock->local)
- llcp_sock->local = NULL
- nfc_put_device(dev)
- dereference llcp_sock->dev
- release_sock()
- return -ERRNO
The nfc_llcp_send_ui_frame() checked llcp_sock->local outside of the
lock, which is racy and ineffective check. Instead, its caller
llcp_sock_sendmsg(), should perform the check inside lock_sock().
Reported-and-tested-by: syzbot+7f23bcddf626e0593a39@syzkaller.appspotmail.com
Fixes: b874dec21d1c ("NFC: Implement LLCP connection less Tx path")
Cc: <stable@vger.kernel.org>
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
---
Changes since v1:
1. Only split to independent set and updating Syzbot tested tag.
---
net/nfc/llcp_sock.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/net/nfc/llcp_sock.c b/net/nfc/llcp_sock.c
index 6cfd30fc0798..0b93a17b9f11 100644
--- a/net/nfc/llcp_sock.c
+++ b/net/nfc/llcp_sock.c
@@ -789,6 +789,11 @@ static int llcp_sock_sendmsg(struct socket *sock, struct msghdr *msg,
lock_sock(sk);
+ if (!llcp_sock->local) {
+ release_sock(sk);
+ return -ENODEV;
+ }
+
if (sk->sk_type == SOCK_DGRAM) {
DECLARE_SOCKADDR(struct sockaddr_nfc_llcp *, addr,
msg->msg_name);
--
2.32.0
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH v2 0/1] nfc: llcp: a fix after syzbot report
2022-01-19 7:48 [PATCH v2 0/1] nfc: llcp: a fix after syzbot report Krzysztof Kozlowski
2022-01-19 7:48 ` [PATCH v2 1/1] nfc: llcp: fix NULL error pointer dereference on sendmsg() after failed bind() Krzysztof Kozlowski
@ 2022-01-19 14:20 ` patchwork-bot+netdevbpf
1 sibling, 0 replies; 3+ messages in thread
From: patchwork-bot+netdevbpf @ 2022-01-19 14:20 UTC (permalink / raw)
To: Krzysztof Kozlowski; +Cc: davem, kuba, linux-nfc, netdev, linux-kernel
Hello:
This patch was applied to netdev/net.git (master)
by David S. Miller <davem@davemloft.net>:
On Wed, 19 Jan 2022 08:48:15 +0100 you wrote:
> Hi,
>
> Syzbot reported an easily reproducible NULL pointer dereference which I was
> struggling to analyze:
> https://syzkaller.appspot.com/bug?extid=7f23bcddf626e0593a39
>
> Although direct fix is obvious, I could not actually find the exact race
> condition scenario leading to it. The patch fixes the issue - at least under
> my QEMU - however all this code looks racy, so I have a feeling I am plumbing
> one leak without fixing root cause.
>
> [...]
Here is the summary with links:
- [v2,1/1] nfc: llcp: fix NULL error pointer dereference on sendmsg() after failed bind()
https://git.kernel.org/netdev/net/c/dded08927ca3
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 3+ messages in thread