[PATCH net v2] NFC: digital: bound SENSF response copy into nfc_target

Pengpeng Hou posted 1 patch 2 months, 1 week ago
net/nfc/digital_technology.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
[PATCH net v2] NFC: digital: bound SENSF response copy into nfc_target
Posted by Pengpeng Hou 2 months, 1 week ago
digital_in_recv_sensf_res() copies the received SENSF response into
struct nfc_target without bounding the copy to target.sensf_res. A full
on-wire digital_sensf_res is 19 bytes long, while nfc_target stores 18
bytes, so oversized or full-length frames can overwrite adjacent stack
fields before digital_target_found() sees the target.

Reject payloads larger than struct digital_sensf_res and clamp the copy
into target.sensf_res so valid 19-byte responses keep working while the
fixed destination buffer stays bounded.

Fixes: 8c0695e4998dd268ff2a05951961247b7e015651 ("NFC Digital: Add NFC-F technology support")
Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
Changes since v1:
- target the net tree and use the NFC: digital: prefix
- add the missing Fixes tag
- preserve valid 19-byte SENSF responses by clamping the copy into
  struct nfc_target
- reject only payloads larger than struct digital_sensf_res

net/nfc/digital_technology.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/net/nfc/digital_technology.c b/net/nfc/digital_technology.c
index 63f1b721c71d..abf544d917f3 100644
--- a/net/nfc/digital_technology.c
+++ b/net/nfc/digital_technology.c
@@ -767,13 +767,18 @@ static void digital_in_recv_sensf_res(struct nfc_digital_dev *ddev, void *arg,
 	}
 
 	skb_pull(resp, 1);
+	if (resp->len > sizeof(struct digital_sensf_res)) {
+		rc = -EIO;
+		goto exit;
+	}
 
 	memset(&target, 0, sizeof(struct nfc_target));
 
 	sensf_res = (struct digital_sensf_res *)resp->data;
 
-	memcpy(target.sensf_res, sensf_res, resp->len);
-	target.sensf_res_len = resp->len;
+	target.sensf_res_len = min_t(unsigned int, resp->len,
+				     sizeof(target.sensf_res));
+	memcpy(target.sensf_res, sensf_res, target.sensf_res_len);
 
 	memcpy(target.nfcid2, sensf_res->nfcid2, NFC_NFCID2_MAXSIZE);
 	target.nfcid2_len = NFC_NFCID2_MAXSIZE;
-- 
2.50.1 (Apple Git-155)
Re: [PATCH net v2] NFC: digital: bound SENSF response copy into nfc_target
Posted by Pengpeng Hou 1 month, 4 weeks ago
Hi Jakub,

Thanks, that makes sense.

I won't resend another bounds-only version as-is. I'll first dig into
why the digital path uses a 19-byte `struct digital_sensf_res` while
the core/UAPI path only carries 18 bytes in `sensf_res`, and follow up
once I have a clearer explanation for what the driver/core boundary
should be here. I'll also shorten the `Fixes:` hash in the next
revision.

Thanks,
Pengpeng
Re: [PATCH net v2] NFC: digital: bound SENSF response copy into nfc_target
Posted by Jakub Kicinski 2 months ago
On Tue, 7 Apr 2026 09:57:36 +0800 Pengpeng Hou wrote:
> digital_in_recv_sensf_res() copies the received SENSF response into
> struct nfc_target without bounding the copy to target.sensf_res. A full
> on-wire digital_sensf_res is 19 bytes long, while nfc_target stores 18
> bytes, so oversized or full-length frames can overwrite adjacent stack
> fields before digital_target_found() sees the target.
> 
> Reject payloads larger than struct digital_sensf_res and clamp the copy
> into target.sensf_res so valid 19-byte responses keep working while the
> fixed destination buffer stays bounded.

You need to solve the riddle why this driver thinks the response is 19
bytes but the core wants to store only 18...

> Fixes: 8c0695e4998dd268ff2a05951961247b7e015651 ("NFC Digital: Add NFC-F technology support")

nit: the hash in the Fixes tag should be only 12 chars
-- 
pw-bot: cr