[PATCH] net/nfc: bound SENSF response copy length

[PATCH] net/nfc: bound SENSF response copy length

[Pengpeng Hou]

`digital_in_recv_sensf_res()` only checks `resp->len` against the
minimum frame size before copying the response into
`target.sensf_res`. The destination is `NFC_SENSF_RES_MAXSIZE` bytes
long, so an oversized SENSF response can overwrite adjacent fields in
the stack-local `struct nfc_target` before the result is handed to
`digital_target_found()`.

Reject frames larger than the destination buffer before copying.

Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
 net/nfc/digital_technology.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/nfc/digital_technology.c b/net/nfc/digital_technology.c
index 63f1b721c71d..8147e61c224a 100644
--- a/net/nfc/digital_technology.c
+++ b/net/nfc/digital_technology.c
@@ -768,6 +768,11 @@ static void digital_in_recv_sensf_res(struct nfc_digital_dev *ddev, void *arg,
 
 	skb_pull(resp, 1);
 
+	if (resp->len > NFC_SENSF_RES_MAXSIZE) {
+		rc = -EIO;
+		goto exit;
+	}
+
 	memset(&target, 0, sizeof(struct nfc_target));
 
 	sensf_res = (struct digital_sensf_res *)resp->data;
-- 
2.50.1 (Apple Git-155)

Re: [PATCH] net/nfc: bound SENSF response copy length

[Simon Horman]

On Sun, Mar 22, 2026 at 11:19:22AM +0800, Pengpeng Hou wrote:
> `digital_in_recv_sensf_res()` only checks `resp->len` against the
> minimum frame size before copying the response into
> `target.sensf_res`. The destination is `NFC_SENSF_RES_MAXSIZE` bytes
> long, so an oversized SENSF response can overwrite adjacent fields in
> the stack-local `struct nfc_target` before the result is handed to
> `digital_target_found()`.
> 
> Reject frames larger than the destination buffer before copying.
> 

Hi Pengpeng Hou,

A fixes tag needs to go here (without a blank line between it
and your Signed-off-by line).

Possibly the following is appropriate

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

> Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>

This patch should be targeted at the net tree, like this:

Subject: [PATCH net] ...

And, looking at git history, it seems that an appropriate prefix
is "NFC: digital:"

Subject: [PATCH net] NFC: digital: bound SENSF response copy length

> ---
>  net/nfc/digital_technology.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/net/nfc/digital_technology.c b/net/nfc/digital_technology.c
> index 63f1b721c71d..8147e61c224a 100644
> --- a/net/nfc/digital_technology.c
> +++ b/net/nfc/digital_technology.c
> @@ -768,6 +768,11 @@ static void digital_in_recv_sensf_res(struct nfc_digital_dev *ddev, void *arg,
>  
>  	skb_pull(resp, 1);
>  
> +	if (resp->len > NFC_SENSF_RES_MAXSIZE) {
> +		rc = -EIO;
> +		goto exit;
> +	}
> +
>  	memset(&target, 0, sizeof(struct nfc_target));
>  
>  	sensf_res = (struct digital_sensf_res *)resp->data;

AI generated review at sashiko.dev says:

    Does this strict bounds check drop valid NFC-F (FeliCa) SENSF_RES
    frames that contain the Request Data (RD) field?

    According to the NFC Forum Digital Protocol, a SENSF_RES frame payload
    can include an optional 2-byte Request Data (RD) field, bringing the
    total length of the response to 19 bytes. This matches the kernel's
    definition of

    struct digital_sensf_res which is exactly 19 bytes long:

    struct digital_sensf_res {
    	u8 cmd;
    	u8 nfcid2[8];
    	u8 pad0[2];
    	u8 pad1[3];
    	u8 mrti_check;
    	u8 mrti_update;
    	u8 pad2;
    	u8 rd[2];
    }

    However, the UAPI macro NFC_SENSF_RES_MAXSIZE is defined as 18.

    If a valid FeliCa SENSF_RES with RD arrives, resp->len will be 19.
    Returning -EIO here completely drops these frames. Could we instead
    clamp the copy length to NFC_SENSF_RES_MAXSIZE (truncating the optional
    RD field if the UAPI buffer cannot be extended) to avoid breaking
    functionality?

Which does seem to be factually correct (please check).
And perhaps the current code buffer overruns if RD is present (in full).
If so it would be interesting to analyse what the effect of that is:
is the neighbouring data unused so this works; or has it always been broken?

Also, I wonder if it is possible to expand NFC_SENSF_RES_MAXSIZE to
accomodate all of a struct nfc_target.

But I am also curious to know how the skb_pull() in the code relates to:

1. Using the packet data as a struct nfc_target after pulling the first byte
2. But checking the length of packet data against
   DIGITAL_SENSF_RES_MIN_LENGTH (17) before the call to skb_pull()