Commit f43ed0c5 authored by Jakub Kicinski's avatar Jakub Kicinski
Browse files

Merge branch 'nfc-fix-leaks-and-races-surfaced-by-nipa'

Jakub Kicinski says:

====================
nfc: fix leaks and races surfaced by NIPA

I recently added the nci test to NIPA. Somewhat surprisingly it runs
without much settup but hits kmemleaks fairly often. Fix a handful of
issues to make the test pass in a stable way.
====================

Link: https://patch.msgid.link/20260303162346.2071888-1-kuba@kernel.org


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parents 40bf00ec d793458c
Loading
Loading
Loading
Loading
+6 −2
Original line number Diff line number Diff line
@@ -707,8 +707,10 @@ static int digital_in_send(struct nfc_dev *nfc_dev, struct nfc_target *target,
	int rc;

	data_exch = kzalloc_obj(*data_exch);
	if (!data_exch)
	if (!data_exch) {
		kfree_skb(skb);
		return -ENOMEM;
	}

	data_exch->cb = cb;
	data_exch->cb_context = cb_context;
@@ -731,8 +733,10 @@ static int digital_in_send(struct nfc_dev *nfc_dev, struct nfc_target *target,
				 data_exch);

exit:
	if (rc)
	if (rc) {
		kfree_skb(skb);
		kfree(data_exch);
	}

	return rc;
}
+16 −2
Original line number Diff line number Diff line
@@ -567,6 +567,10 @@ static int nci_close_device(struct nci_dev *ndev)
		flush_workqueue(ndev->cmd_wq);
		timer_delete_sync(&ndev->cmd_timer);
		timer_delete_sync(&ndev->data_timer);
		if (test_bit(NCI_DATA_EXCHANGE, &ndev->flags))
			nci_data_exchange_complete(ndev, NULL,
						   ndev->cur_conn_id,
						   -ENODEV);
		mutex_unlock(&ndev->req_lock);
		return 0;
	}
@@ -598,6 +602,11 @@ static int nci_close_device(struct nci_dev *ndev)
	flush_workqueue(ndev->cmd_wq);

	timer_delete_sync(&ndev->cmd_timer);
	timer_delete_sync(&ndev->data_timer);

	if (test_bit(NCI_DATA_EXCHANGE, &ndev->flags))
		nci_data_exchange_complete(ndev, NULL, ndev->cur_conn_id,
					   -ENODEV);

	/* Clear flags except NCI_UNREG */
	ndev->flags &= BIT(NCI_UNREG);
@@ -1035,18 +1044,23 @@ static int nci_transceive(struct nfc_dev *nfc_dev, struct nfc_target *target,
	struct nci_conn_info *conn_info;

	conn_info = ndev->rf_conn_info;
	if (!conn_info)
	if (!conn_info) {
		kfree_skb(skb);
		return -EPROTO;
	}

	pr_debug("target_idx %d, len %d\n", target->idx, skb->len);

	if (!ndev->target_active_prot) {
		pr_err("unable to exchange data, no active target\n");
		kfree_skb(skb);
		return -EINVAL;
	}

	if (test_and_set_bit(NCI_DATA_EXCHANGE, &ndev->flags))
	if (test_and_set_bit(NCI_DATA_EXCHANGE, &ndev->flags)) {
		kfree_skb(skb);
		return -EBUSY;
	}

	/* store cb and context to be used on receiving data */
	conn_info->data_exchange_cb = cb;
+8 −4
Original line number Diff line number Diff line
@@ -33,7 +33,8 @@ void nci_data_exchange_complete(struct nci_dev *ndev, struct sk_buff *skb,
	conn_info = nci_get_conn_info_by_conn_id(ndev, conn_id);
	if (!conn_info) {
		kfree_skb(skb);
		goto exit;
		clear_bit(NCI_DATA_EXCHANGE, &ndev->flags);
		return;
	}

	cb = conn_info->data_exchange_cb;
@@ -45,6 +46,12 @@ void nci_data_exchange_complete(struct nci_dev *ndev, struct sk_buff *skb,
	timer_delete_sync(&ndev->data_timer);
	clear_bit(NCI_DATA_EXCHANGE_TO, &ndev->flags);

	/* Mark the exchange as done before calling the callback.
	 * The callback (e.g. rawsock_data_exchange_complete) may
	 * want to immediately queue another data exchange.
	 */
	clear_bit(NCI_DATA_EXCHANGE, &ndev->flags);

	if (cb) {
		/* forward skb to nfc core */
		cb(cb_context, skb, err);
@@ -54,9 +61,6 @@ void nci_data_exchange_complete(struct nci_dev *ndev, struct sk_buff *skb,
		/* no waiting callback, free skb */
		kfree_skb(skb);
	}

exit:
	clear_bit(NCI_DATA_EXCHANGE, &ndev->flags);
}

/* ----------------- NCI TX Data ----------------- */
+11 −0
Original line number Diff line number Diff line
@@ -67,6 +67,17 @@ static int rawsock_release(struct socket *sock)
	if (sock->type == SOCK_RAW)
		nfc_sock_unlink(&raw_sk_list, sk);

	if (sk->sk_state == TCP_ESTABLISHED) {
		/* Prevent rawsock_tx_work from starting new transmits and
		 * wait for any in-progress work to finish.  This must happen
		 * before the socket is orphaned to avoid a race where
		 * rawsock_tx_work runs after the NCI device has been freed.
		 */
		sk->sk_shutdown |= SEND_SHUTDOWN;
		cancel_work_sync(&nfc_rawsock(sk)->tx_work);
		rawsock_write_queue_purge(sk);
	}

	sock_orphan(sk);
	sock_put(sk);