[PATCH net v2] nfc: nci: avoid unbounded skb allocation when max_pkt_payload_len is zero

[Liu Chao]

nci_queue_tx_data_frags() uses conn_info->max_pkt_payload_len as the
fragment size.  When that value is zero, frag_len is always zero and
total_len never decreases.  The loop then allocates skbs without
bound: none of them are freed inside the loop, they accumulate on
frags_q, and there is no cond_resched() in the loop body.  A single
sendmsg() can therefore consume all allocatable memory, and on
CONFIG_PREEMPT_NONE it occupies the CPU long enough to trip the
softlockup watchdog:

  watchdog: BUG: soft lockup - CPU#3 stuck for 26s! [kworker/3:1:57]
  Workqueue: events rawsock_tx_work [nfc]
  Call Trace:
   nci_send_data+0x1ca/0x6b0 [nci]
   nci_transceive+0xbb/0x170 [nci]
   rawsock_tx_work+0xb5/0x1a0 [nfc]

max_pkt_payload_len comes straight from controller-supplied fields,
with no check for zero:

  ntf.c: conn_info->max_pkt_payload_len = ntf.max_data_pkt_payload_size;
  rsp.c: conn_info->max_pkt_payload_len = rsp->max_ctrl_pkt_payload_len;

Reject the zero value in the fragmentation path rather than at the
assignment sites.  nci_queue_tx_data_frags() is the only place that
loops over the RF data path's conn_info, and nci_send_data() takes
the non-fragmenting branch only for skb->len <= max_pkt_payload_len,
which for a zero limit means empty skbs alone.  Validating on
assignment would not be sufficient either, because
nci_rf_disc_rsp_packet() allocates ndev->rf_conn_info with
devm_kzalloc(), so max_pkt_payload_len is already zero before any
notification arrives.

No legitimate configuration is affected: where the NCI spec does
mandate a zero Max Data Packet Payload Size -- the NFCEE Direct RF
Interface -- nci_rf_intf_activated_ntf_packet() takes the "goto
listen" shortcut, bypassing the assignment entirely.

Snapshot the field once with READ_ONCE() and use the snapshot for
both the check and the min_t() bound: the rx workqueue updates the
field without any lock held against this path, so without the
snapshot the check could validate a value the loop no longer
consumes.

nci_hci_send_data() also loops over the same field, but on a
different conn_info instance (ndev->hci_dev->conn_info) created by
nci_core_conn_create_rsp_packet(); a zero or one there underflows
the loop arithmetic and will be addressed in a separate patch.  This
guards the path carrying the reported bug.

Fixes: 6a2968aaf50c ("NFC: basic NCI protocol implementation")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Liu Chao <liuc63@xxxxxxxxxxxx>

---

v1 claimed that nci_queue_tx_data_frags() is "the only place that
loops" over max_pkt_payload_len.  That holds for the RF data path
(ndev->rf_conn_info) but was overstated as a blanket claim: the
Sashiko review of v1 pointed out that nci_hci_send_data() loops over
the same field as well, albeit on a separate conn_info instance
(ndev->hci_dev->conn_info) -- pre-existing and unchanged here; it
will be fixed separately.

The READ_ONCE() snapshot follows the same review: with the check and
the loop reading the field independently, a store from the rx
workqueue in between would let the loop spin on a value the check
had just rejected.
---
 net/nfc/nci/data.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/net/nfc/nci/data.c b/net/nfc/nci/data.c
index 4253edea5..eeb5260c5 100644
--- a/net/nfc/nci/data.c
+++ b/net/nfc/nci/data.c
@@ -104,6 +104,7 @@ static int nci_queue_tx_data_frags(struct nci_dev *ndev,
 	struct sk_buff_head frags_q;
 	struct sk_buff *skb_frag;
 	int frag_len;
+	u8 max_len;
 	int rc = 0;
 
 	pr_debug("conn_id 0x%x, total_len %d\n", conn_id, total_len);
@@ -114,11 +115,18 @@ static int nci_queue_tx_data_frags(struct nci_dev *ndev,
 		goto exit;
 	}
 
+	/* the rx workqueue may update the field concurrently */
+	max_len = READ_ONCE(conn_info->max_pkt_payload_len);
+
+	if (!max_len) {
+		rc = -EPROTO;
+		goto exit;
+	}
+
 	__skb_queue_head_init(&frags_q);
 
 	while (total_len) {
-		frag_len =
-			min_t(int, total_len, conn_info->max_pkt_payload_len);
+		frag_len = min_t(int, total_len, max_len);
 
 		skb_frag = nci_skb_alloc(ndev,
 					 (NCI_DATA_HDR_SIZE + frag_len),
-- 
2.50.1