Skip to content

Commit 9c328f5

Browse files
iwerethPaolo Abeni
iwereth
authored and
Paolo Abeni
committed
net: nfc: nci: Add parameter validation for packet data
Syzbot reported an uninitialized value bug in nci_init_req, which was introduced by commit 5aca796 ("Merge tag 'perf-tools-fixes-for-v6.17-2025-09-16' of git://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools"). This bug arises due to very limited and poor input validation that was done at nic_valid_size(). 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 at `nic_valid_size()`, add validation of the data in the respective handlers and return error values in case of failure. Release the skb if error values are returned from handlers in `nci_nft_packet` and effectively do a silent drop 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: [email protected] Closes: https://syzkaller.appspot.com/bug?extid=740e04c2a93467a0f8c8 Fixes: 6a2968a ("NFC: basic NCI protocol implementation") Tested-by: [email protected] Cc: [email protected] Signed-off-by: Deepak Sharma <[email protected]> Reviewed-by: Vadim Fedorenko <[email protected]> Link: https://patch.msgid.link/[email protected] Signed-off-by: Paolo Abeni <[email protected]>
1 parent 99e4c35 commit 9c328f5

File tree

1 file changed

+99
-36
lines changed

1 file changed

+99
-36
lines changed

‎net/nfc/nci/ntf.c‎

Lines changed: 99 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,16 @@
2727

2828
/* Handle NCI Notification packets */
2929

30-
static void nci_core_reset_ntf_packet(struct nci_dev *ndev,
31-
const struct sk_buff *skb)
30+
static int nci_core_reset_ntf_packet(struct nci_dev *ndev,
31+
const struct sk_buff *skb)
3232
{
3333
/* Handle NCI 2.x core reset notification */
34-
const struct nci_core_reset_ntf *ntf = (void *)skb->data;
34+
const struct nci_core_reset_ntf *ntf;
35+
36+
if (skb->len < sizeof(struct nci_core_reset_ntf))
37+
return -EINVAL;
38+
39+
ntf = (struct nci_core_reset_ntf *)skb->data;
3540

3641
ndev->nci_ver = ntf->nci_ver;
3742
pr_debug("nci_ver 0x%x, config_status 0x%x\n",
@@ -42,15 +47,22 @@ static void nci_core_reset_ntf_packet(struct nci_dev *ndev,
4247
__le32_to_cpu(ntf->manufact_specific_info);
4348

4449
nci_req_complete(ndev, NCI_STATUS_OK);
50+
51+
return 0;
4552
}
4653

47-
static void nci_core_conn_credits_ntf_packet(struct nci_dev *ndev,
48-
struct sk_buff *skb)
54+
static int nci_core_conn_credits_ntf_packet(struct nci_dev *ndev,
55+
struct sk_buff *skb)
4956
{
50-
struct nci_core_conn_credit_ntf *ntf = (void *) skb->data;
57+
struct nci_core_conn_credit_ntf *ntf;
5158
struct nci_conn_info *conn_info;
5259
int i;
5360

61+
if (skb->len < sizeof(struct nci_core_conn_credit_ntf))
62+
return -EINVAL;
63+
64+
ntf = (struct nci_core_conn_credit_ntf *)skb->data;
65+
5466
pr_debug("num_entries %d\n", ntf->num_entries);
5567

5668
if (ntf->num_entries > NCI_MAX_NUM_CONN)
@@ -68,7 +80,7 @@ static void nci_core_conn_credits_ntf_packet(struct nci_dev *ndev,
6880
conn_info = nci_get_conn_info_by_conn_id(ndev,
6981
ntf->conn_entries[i].conn_id);
7082
if (!conn_info)
71-
return;
83+
return 0;
7284

7385
atomic_add(ntf->conn_entries[i].credits,
7486
&conn_info->credits_cnt);
@@ -77,12 +89,19 @@ static void nci_core_conn_credits_ntf_packet(struct nci_dev *ndev,
7789
/* trigger the next tx */
7890
if (!skb_queue_empty(&ndev->tx_q))
7991
queue_work(ndev->tx_wq, &ndev->tx_work);
92+
93+
return 0;
8094
}
8195

82-
static void nci_core_generic_error_ntf_packet(struct nci_dev *ndev,
83-
const struct sk_buff *skb)
96+
static int nci_core_generic_error_ntf_packet(struct nci_dev *ndev,
97+
const struct sk_buff *skb)
8498
{
85-
__u8 status = skb->data[0];
99+
__u8 status;
100+
101+
if (skb->len < 1)
102+
return -EINVAL;
103+
104+
status = skb->data[0];
86105

87106
pr_debug("status 0x%x\n", status);
88107

@@ -91,12 +110,19 @@ static void nci_core_generic_error_ntf_packet(struct nci_dev *ndev,
91110
(the state remains the same) */
92111
nci_req_complete(ndev, status);
93112
}
113+
114+
return 0;
94115
}
95116

96-
static void nci_core_conn_intf_error_ntf_packet(struct nci_dev *ndev,
97-
struct sk_buff *skb)
117+
static int nci_core_conn_intf_error_ntf_packet(struct nci_dev *ndev,
118+
struct sk_buff *skb)
98119
{
99-
struct nci_core_intf_error_ntf *ntf = (void *) skb->data;
120+
struct nci_core_intf_error_ntf *ntf;
121+
122+
if (skb->len < sizeof(struct nci_core_intf_error_ntf))
123+
return -EINVAL;
124+
125+
ntf = (struct nci_core_intf_error_ntf *)skb->data;
100126

101127
ntf->conn_id = nci_conn_id(&ntf->conn_id);
102128

@@ -105,6 +131,8 @@ static void nci_core_conn_intf_error_ntf_packet(struct nci_dev *ndev,
105131
/* complete the data exchange transaction, if exists */
106132
if (test_bit(NCI_DATA_EXCHANGE, &ndev->flags))
107133
nci_data_exchange_complete(ndev, NULL, ntf->conn_id, -EIO);
134+
135+
return 0;
108136
}
109137

110138
static const __u8 *
@@ -329,13 +357,18 @@ void nci_clear_target_list(struct nci_dev *ndev)
329357
ndev->n_targets = 0;
330358
}
331359

332-
static void nci_rf_discover_ntf_packet(struct nci_dev *ndev,
333-
const struct sk_buff *skb)
360+
static int nci_rf_discover_ntf_packet(struct nci_dev *ndev,
361+
const struct sk_buff *skb)
334362
{
335363
struct nci_rf_discover_ntf ntf;
336-
const __u8 *data = skb->data;
364+
const __u8 *data;
337365
bool add_target = true;
338366

367+
if (skb->len < sizeof(struct nci_rf_discover_ntf))
368+
return -EINVAL;
369+
370+
data = skb->data;
371+
339372
ntf.rf_discovery_id = *data++;
340373
ntf.rf_protocol = *data++;
341374
ntf.rf_tech_and_mode = *data++;
@@ -390,6 +423,8 @@ static void nci_rf_discover_ntf_packet(struct nci_dev *ndev,
390423
nfc_targets_found(ndev->nfc_dev, ndev->targets,
391424
ndev->n_targets);
392425
}
426+
427+
return 0;
393428
}
394429

395430
static int nci_extract_activation_params_iso_dep(struct nci_dev *ndev,
@@ -553,14 +588,19 @@ static int nci_store_ats_nfc_iso_dep(struct nci_dev *ndev,
553588
return NCI_STATUS_OK;
554589
}
555590

556-
static void nci_rf_intf_activated_ntf_packet(struct nci_dev *ndev,
557-
const struct sk_buff *skb)
591+
static int nci_rf_intf_activated_ntf_packet(struct nci_dev *ndev,
592+
const struct sk_buff *skb)
558593
{
559594
struct nci_conn_info *conn_info;
560595
struct nci_rf_intf_activated_ntf ntf;
561-
const __u8 *data = skb->data;
596+
const __u8 *data;
562597
int err = NCI_STATUS_OK;
563598

599+
if (skb->len < sizeof(struct nci_rf_intf_activated_ntf))
600+
return -EINVAL;
601+
602+
data = skb->data;
603+
564604
ntf.rf_discovery_id = *data++;
565605
ntf.rf_interface = *data++;
566606
ntf.rf_protocol = *data++;
@@ -667,7 +707,7 @@ static void nci_rf_intf_activated_ntf_packet(struct nci_dev *ndev,
667707
if (err == NCI_STATUS_OK) {
668708
conn_info = ndev->rf_conn_info;
669709
if (!conn_info)
670-
return;
710+
return 0;
671711

672712
conn_info->max_pkt_payload_len = ntf.max_data_pkt_payload_size;
673713
conn_info->initial_num_credits = ntf.initial_num_credits;
@@ -721,19 +761,26 @@ static void nci_rf_intf_activated_ntf_packet(struct nci_dev *ndev,
721761
pr_err("error when signaling tm activation\n");
722762
}
723763
}
764+
765+
return 0;
724766
}
725767

726-
static void nci_rf_deactivate_ntf_packet(struct nci_dev *ndev,
727-
const struct sk_buff *skb)
768+
static int nci_rf_deactivate_ntf_packet(struct nci_dev *ndev,
769+
const struct sk_buff *skb)
728770
{
729771
const struct nci_conn_info *conn_info;
730-
const struct nci_rf_deactivate_ntf *ntf = (void *)skb->data;
772+
const struct nci_rf_deactivate_ntf *ntf;
773+
774+
if (skb->len < sizeof(struct nci_rf_deactivate_ntf))
775+
return -EINVAL;
776+
777+
ntf = (struct nci_rf_deactivate_ntf *)skb->data;
731778

732779
pr_debug("entry, type 0x%x, reason 0x%x\n", ntf->type, ntf->reason);
733780

734781
conn_info = ndev->rf_conn_info;
735782
if (!conn_info)
736-
return;
783+
return 0;
737784

738785
/* drop tx data queue */
739786
skb_queue_purge(&ndev->tx_q);
@@ -765,14 +812,20 @@ static void nci_rf_deactivate_ntf_packet(struct nci_dev *ndev,
765812
}
766813

767814
nci_req_complete(ndev, NCI_STATUS_OK);
815+
816+
return 0;
768817
}
769818

770-
static void nci_nfcee_discover_ntf_packet(struct nci_dev *ndev,
771-
const struct sk_buff *skb)
819+
static int nci_nfcee_discover_ntf_packet(struct nci_dev *ndev,
820+
const struct sk_buff *skb)
772821
{
773822
u8 status = NCI_STATUS_OK;
774-
const struct nci_nfcee_discover_ntf *nfcee_ntf =
775-
(struct nci_nfcee_discover_ntf *)skb->data;
823+
const struct nci_nfcee_discover_ntf *nfcee_ntf;
824+
825+
if (skb->len < sizeof(struct nci_nfcee_discover_ntf))
826+
return -EINVAL;
827+
828+
nfcee_ntf = (struct nci_nfcee_discover_ntf *)skb->data;
776829

777830
/* NFCForum NCI 9.2.1 HCI Network Specific Handling
778831
* If the NFCC supports the HCI Network, it SHALL return one,
@@ -783,6 +836,8 @@ static void nci_nfcee_discover_ntf_packet(struct nci_dev *ndev,
783836
ndev->cur_params.id = nfcee_ntf->nfcee_id;
784837

785838
nci_req_complete(ndev, status);
839+
840+
return 0;
786841
}
787842

788843
void nci_ntf_packet(struct nci_dev *ndev, struct sk_buff *skb)
@@ -809,35 +864,43 @@ void nci_ntf_packet(struct nci_dev *ndev, struct sk_buff *skb)
809864

810865
switch (ntf_opcode) {
811866
case NCI_OP_CORE_RESET_NTF:
812-
nci_core_reset_ntf_packet(ndev, skb);
867+
if (nci_core_reset_ntf_packet(ndev, skb))
868+
goto end;
813869
break;
814870

815871
case NCI_OP_CORE_CONN_CREDITS_NTF:
816-
nci_core_conn_credits_ntf_packet(ndev, skb);
872+
if (nci_core_conn_credits_ntf_packet(ndev, skb))
873+
goto end;
817874
break;
818875

819876
case NCI_OP_CORE_GENERIC_ERROR_NTF:
820-
nci_core_generic_error_ntf_packet(ndev, skb);
877+
if (nci_core_generic_error_ntf_packet(ndev, skb))
878+
goto end;
821879
break;
822880

823881
case NCI_OP_CORE_INTF_ERROR_NTF:
824-
nci_core_conn_intf_error_ntf_packet(ndev, skb);
882+
if (nci_core_conn_intf_error_ntf_packet(ndev, skb))
883+
goto end;
825884
break;
826885

827886
case NCI_OP_RF_DISCOVER_NTF:
828-
nci_rf_discover_ntf_packet(ndev, skb);
887+
if (nci_rf_discover_ntf_packet(ndev, skb))
888+
goto end;
829889
break;
830890

831891
case NCI_OP_RF_INTF_ACTIVATED_NTF:
832-
nci_rf_intf_activated_ntf_packet(ndev, skb);
892+
if (nci_rf_intf_activated_ntf_packet(ndev, skb))
893+
goto end;
833894
break;
834895

835896
case NCI_OP_RF_DEACTIVATE_NTF:
836-
nci_rf_deactivate_ntf_packet(ndev, skb);
897+
if (nci_rf_deactivate_ntf_packet(ndev, skb))
898+
goto end;
837899
break;
838900

839901
case NCI_OP_NFCEE_DISCOVER_NTF:
840-
nci_nfcee_discover_ntf_packet(ndev, skb);
902+
if (nci_nfcee_discover_ntf_packet(ndev, skb))
903+
goto end;
841904
break;
842905

843906
case NCI_OP_RF_NFCEE_ACTION_NTF:

0 commit comments

Comments
 (0)