| Message ID | 20240320005412.905060-1-ryasuoka@redhat.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | d24b03535e5eb82e025219c2f632b485409c898f |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net,v2] nfc: nci: Fix uninit-value in nci_dev_up and nci_ntf_packet | expand |
Wed, Mar 20, 2024 at 01:54:10AM CET, ryasuoka@redhat.com wrote: >syzbot reported the following uninit-value access issue [1][2]: > >nci_rx_work() parses and processes received packet. When the payload >length is zero, each message type handler reads uninitialized payload >and KMSAN detects this issue. The receipt of a packet with a zero-size >payload is considered unexpected, and therefore, such packets should be >silently discarded. > >This patch resolved this issue by checking payload size before calling >each message type handler codes. Nit. Instead of talking about "this patch" in this patch description, you should use imperative mood to tell the codebase what to do. https://www.kernel.org/doc/html/v6.6/process/submitting-patches.html#describe-your-changes Patch looks ok. > >Fixes: 6a2968aaf50c ("NFC: basic NCI protocol implementation") >Reported-and-tested-by: syzbot+7ea9413ea6749baf5574@syzkaller.appspotmail.com >Reported-and-tested-by: syzbot+29b5ca705d2e0f4a44d2@syzkaller.appspotmail.com >Closes: https://syzkaller.appspot.com/bug?extid=7ea9413ea6749baf5574 [1] >Closes: https://syzkaller.appspot.com/bug?extid=29b5ca705d2e0f4a44d2 [2] >Signed-off-by: Ryosuke Yasuoka <ryasuoka@redhat.com> >--- > >v2 >- Fix typo in commit message >- Remove Call Trace from commit message that syzbot reported. Make it > shorter than the previous version. >- Check the payload length in earlier code path. And it can address > another reported syzbot bug too. [2] > >v1 >https://lore.kernel.org/all/20240312145658.417288-1-ryasuoka@redhat.com/ > > > net/nfc/nci/core.c | 5 +++++ > 1 file changed, 5 insertions(+) > >diff --git a/net/nfc/nci/core.c b/net/nfc/nci/core.c >index 6c9592d05120..f471fc54c6a1 100644 >--- a/net/nfc/nci/core.c >+++ b/net/nfc/nci/core.c >@@ -1512,6 +1512,11 @@ static void nci_rx_work(struct work_struct *work) > nfc_send_to_raw_sock(ndev->nfc_dev, skb, > RAW_PAYLOAD_NCI, NFC_DIRECTION_RX); > >+ if (!nci_plen(skb->data)) { >+ kfree_skb(skb); >+ break; >+ } >+ > /* Process frame */ > switch (nci_mt(skb->data)) { > case NCI_MT_RSP_PKT: >-- >2.44.0 > >
On Tue, Mar 19, 2024, at 8:54 PM, Ryosuke Yasuoka wrote: > syzbot reported the following uninit-value access issue [1][2]: > > nci_rx_work() parses and processes received packet. When the payload > length is zero, each message type handler reads uninitialized payload > and KMSAN detects this issue. The receipt of a packet with a zero-size > payload is considered unexpected, and therefore, such packets should be > silently discarded. > > This patch resolved this issue by checking payload size before calling > each message type handler codes. > > Fixes: 6a2968aaf50c ("NFC: basic NCI protocol implementation") > Reported-and-tested-by: syzbot+7ea9413ea6749baf5574@syzkaller.appspotmail.com > Reported-and-tested-by: syzbot+29b5ca705d2e0f4a44d2@syzkaller.appspotmail.com > Closes: https://syzkaller.appspot.com/bug?extid=7ea9413ea6749baf5574 [1] > Closes: https://syzkaller.appspot.com/bug?extid=29b5ca705d2e0f4a44d2 [2] > Signed-off-by: Ryosuke Yasuoka <ryasuoka@redhat.com> > --- For what it's worth, Reviewed-by: Jeremy Cline <jeremy@jcline.org>
On 20/03/2024 01:54, Ryosuke Yasuoka wrote: > syzbot reported the following uninit-value access issue [1][2]: > > nci_rx_work() parses and processes received packet. When the payload > length is zero, each message type handler reads uninitialized payload > and KMSAN detects this issue. The receipt of a packet with a zero-size > payload is considered unexpected, and therefore, such packets should be > silently discarded. > > This patch resolved this issue by checking payload size before calling > each message type handler codes. > > Fixes: 6a2968aaf50c ("NFC: basic NCI protocol implementation") > Reported-and-tested-by: syzbot+7ea9413ea6749baf5574@syzkaller.appspotmail.com > Reported-and-tested-by: syzbot+29b5ca705d2e0f4a44d2@syzkaller.appspotmail.com > Closes: https://syzkaller.appspot.com/bug?extid=7ea9413ea6749baf5574 [1] > Closes: https://syzkaller.appspot.com/bug?extid=29b5ca705d2e0f4a44d2 [2] > Signed-off-by: Ryosuke Yasuoka <ryasuoka@redhat.com> > --- > > v2 > - Fix typo in commit message > - Remove Call Trace from commit message that syzbot reported. Make it > shorter than the previous version. > - Check the payload length in earlier code path. And it can address > another reported syzbot bug too. [2] Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Best regards, Krzysztof
Hello: This patch was applied to netdev/net.git (main) by David S. Miller <davem@davemloft.net>: On Wed, 20 Mar 2024 09:54:10 +0900 you wrote: > syzbot reported the following uninit-value access issue [1][2]: > > nci_rx_work() parses and processes received packet. When the payload > length is zero, each message type handler reads uninitialized payload > and KMSAN detects this issue. The receipt of a packet with a zero-size > payload is considered unexpected, and therefore, such packets should be > silently discarded. > > [...] Here is the summary with links: - [net,v2] nfc: nci: Fix uninit-value in nci_dev_up and nci_ntf_packet https://git.kernel.org/netdev/net/c/d24b03535e5e You are awesome, thank you!
diff --git a/net/nfc/nci/core.c b/net/nfc/nci/core.c index 6c9592d05120..f471fc54c6a1 100644 --- a/net/nfc/nci/core.c +++ b/net/nfc/nci/core.c @@ -1512,6 +1512,11 @@ static void nci_rx_work(struct work_struct *work) nfc_send_to_raw_sock(ndev->nfc_dev, skb, RAW_PAYLOAD_NCI, NFC_DIRECTION_RX); + if (!nci_plen(skb->data)) { + kfree_skb(skb); + break; + } + /* Process frame */ switch (nci_mt(skb->data)) { case NCI_MT_RSP_PKT:
syzbot reported the following uninit-value access issue [1][2]: nci_rx_work() parses and processes received packet. When the payload length is zero, each message type handler reads uninitialized payload and KMSAN detects this issue. The receipt of a packet with a zero-size payload is considered unexpected, and therefore, such packets should be silently discarded. This patch resolved this issue by checking payload size before calling each message type handler codes. Fixes: 6a2968aaf50c ("NFC: basic NCI protocol implementation") Reported-and-tested-by: syzbot+7ea9413ea6749baf5574@syzkaller.appspotmail.com Reported-and-tested-by: syzbot+29b5ca705d2e0f4a44d2@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=7ea9413ea6749baf5574 [1] Closes: https://syzkaller.appspot.com/bug?extid=29b5ca705d2e0f4a44d2 [2] Signed-off-by: Ryosuke Yasuoka <ryasuoka@redhat.com> --- v2 - Fix typo in commit message - Remove Call Trace from commit message that syzbot reported. Make it shorter than the previous version. - Check the payload length in earlier code path. And it can address another reported syzbot bug too. [2] v1 https://lore.kernel.org/all/20240312145658.417288-1-ryasuoka@redhat.com/ net/nfc/nci/core.c | 5 +++++ 1 file changed, 5 insertions(+)