Syzbot reported an uninit-value bug at nci_init_req for commit 5aca7966d2a7 This bug arises due to very limited and poor input validation that was done at net/nfc/nci/core.c:1543. This validation only validates the skb->len (directly reflects size provided at the userspace interface) with the length provided in the buffer itself (interpreted as NCI_HEADER). This leads to the processing of memory content at the address assuming the correct layout per what opcode requires there. This leads to the accesses to buffer of `skb_buff->data` which is not assigned anything yet Following the same silent drop of packets of invalid sizes, I have added validation in the `nci_nft_packet` which processes NFT packets and silently return in case of failure of any validation check Possible TODO: because we silently drop the packets, the call to `nci_request` will be waiting for completion of request and will face timeouts. These timeouts can get excessively logged in the dmesg. A proper handling of them may require to export `nci_request_cancel` (or propagate error handling from the nft packets handlers) Reported-by: syzbot+740e04c2a93467a0f8c8@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=740e04c2a93467a0f8c8 Signed-off-by: Deepak Sharma --- net/nfc/nci/ntf.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/net/nfc/nci/ntf.c b/net/nfc/nci/ntf.c index a818eff27e6b..05ee474a1068 100644 --- a/net/nfc/nci/ntf.c +++ b/net/nfc/nci/ntf.c @@ -809,34 +809,52 @@ void nci_ntf_packet(struct nci_dev *ndev, struct sk_buff *skb) switch (ntf_opcode) { case NCI_OP_CORE_RESET_NTF: + if (skb->len < sizeof(struct nci_core_reset_ntf)) + return; nci_core_reset_ntf_packet(ndev, skb); break; case NCI_OP_CORE_CONN_CREDITS_NTF: + if (skb->len < sizeof(struct nci_core_conn_credit_ntf)) + return; nci_core_conn_credits_ntf_packet(ndev, skb); break; case NCI_OP_CORE_GENERIC_ERROR_NTF: + if (skb->len < 1) + return; nci_core_generic_error_ntf_packet(ndev, skb); break; case NCI_OP_CORE_INTF_ERROR_NTF: + if (skb->len < sizeof(struct nci_core_intf_error_ntf)) + return; nci_core_conn_intf_error_ntf_packet(ndev, skb); break; case NCI_OP_RF_DISCOVER_NTF: + // tech specific params are included as unions + if (skb->len < sizeof(struct nci_rf_discover_ntf)) + return; nci_rf_discover_ntf_packet(ndev, skb); break; case NCI_OP_RF_INTF_ACTIVATED_NTF: + // tech specific params are included as unions + if (skb->len < sizeof(struct nci_rf_intf_activated_ntf)) + return; nci_rf_intf_activated_ntf_packet(ndev, skb); break; case NCI_OP_RF_DEACTIVATE_NTF: + if (skb->len < sizeof(struct nci_rf_deactivate_ntf)) + return; nci_rf_deactivate_ntf_packet(ndev, skb); break; case NCI_OP_NFCEE_DISCOVER_NTF: + if (skb->len < sizeof(struct nci_nfcee_discover_ntf)) + return; nci_nfcee_discover_ntf_packet(ndev, skb); break; -- 2.51.0