Patch 1 fixes a regression: since commit 8265a626cc14 ("nfc: nci: fix
double completion race in nci_data_exchange_complete") every data packet
received on an NCI logical connection other than the static RF one is
dropped, because only nci_transceive() sets NCI_DATA_EXCHANGE. That
breaks HCI over NCI (st-nci) and nci_nfcc_loopback() entirely: every
command times out. The commit is in mainline and in the 5.10+ stable
trees.
Patches 2 and 3 fix the handling of the stored response in those two
paths. conn_info->rx_skb is never cleared after the response has been
handed to the caller and freed, and a request can be completed without
a new response (an HCP event or command received on the pipe, or an
unrelated NCI response). The next command then reads, modifies and
returns the freed skb, or dereferences NULL.
Patches 4 and 5 bound the controller supplied pipe ids used as an index
into the pipes[] array in the st21nfca and st-nci load_session
handlers, as pointed out by sashiko.
Testing:
- Patches 1-3 were tested on arm64 QEMU with KASAN, using virtual_ncidev
and a userspace NFCC emulator, plus a test-only ioctl in
virtual_ncidev (not part of this series) that drives
nci_core_conn_create(), nci_hci_dev_session_init(),
nci_hci_get_param() and nci_nfcc_loopback() the way st-nci does.
- Without patch 1, the response to the first ANY_OPEN_PIPE is dropped
and nci_hci_dev_session_init() returns -ETIMEDOUT; with it, session
init and HCP commands complete.
- Without patches 2/3: slab-use-after-free in nci_hci_get_param()
(HCP event instead of a response), null-ptr-deref in
nci_hci_get_param() (no response stored yet), and a
slab-use-after-free on the skb returned by the second
nci_nfcc_loopback(). With them all three return -EPROTO, normal
responses are still returned, and KASAN is quiet.
- tools/testing/selftests/nci passes (8/8) with the series applied.
- Patches 4 and 5 are compile tested only (W=1); I have no ST hardware.
Sanghyun Park's pending "nfc: nci: Fix conn_info use-after-free" v5
touches the same functions in hci.c (it adds a NULL check for rx_skb,
but does not clear it); whichever goes in second will need a rebase.
Joas Antonio dos Santos (5):
nfc: nci: don't drop data received on HCI and loopback connections
nfc: nci: hci: consume the HCP response exactly once
nfc: nci: consume the loopback response exactly once
nfc: st21nfca: validate pipe ids in st21nfca_hci_load_session()
nfc: st-nci: validate pipe ids in st_nci_hci_load_session()
drivers/nfc/st-nci/se.c | 8 ++++
drivers/nfc/st21nfca/core.c | 8 ++++
net/nfc/nci/core.c | 18 ++++++--
net/nfc/nci/data.c | 9 +++-
net/nfc/nci/hci.c | 83 ++++++++++++++++++++++---------------
5 files changed, 87 insertions(+), 39 deletions(-)
--
2.43.0