Re: [PATCH net-next v2 2/2] nfc: s3fwrn5: support the S3NRN4V variant

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Preexisting out-of-bounds read in s3fwrn5_nci_rf_configure during checksum calculation.
--

commit d87946b70fa655f2c45f22293e0eb74e7a2f637d
Author: Jorijn van der Graaf <jorijnvdgraaf@catcrafts.net>

nfc: s3fwrn5: support the S3NRN4V variant

This patch adds support for the S3NRN4V NFC controller, bypassing the firmware
download step and introducing a new dual-option mechanism for loading RF
registers. It also configures the reference clock via FW_CFG and manages an
optional CLK_REQ handshake.

> diff --git a/drivers/nfc/s3fwrn5/nci.c b/drivers/nfc/s3fwrn5/nci.c
> index 5a9de11bbecea..7034fb810e18a 100644
> --- a/drivers/nfc/s3fwrn5/nci.c
> +++ b/drivers/nfc/s3fwrn5/nci.c

[ ... ]

> +int s3fwrn5_nci_rf_configure_dual(struct s3fwrn5_info *info,
> +				  const char *hw_name, const char *sw_name)
> +{
> +	const struct firmware *hw_fw = NULL, *sw_fw = NULL;

[ ... ]

> +	/*
> +	 * The stream is checksummed as 32-bit words and pushed in at most 256
> +	 * sections (the section index is a single byte); reject blobs that
> +	 * would silently break either.
> +	 */
> +	if (merged_size % 4 ||
> +	    merged_size > 256 * NCI_PROP_DUAL_SECTION_SIZE) {
> +		dev_err(dev, "invalid rfreg blob size (%zu)\n", merged_size);
> +		ret = -EINVAL;
> +		goto out;
> +	}

[Severity: High]
This isn't a bug introduced by this patch, but while you correctly validate
the firmware size in this new s3fwrn5_nci_rf_configure_dual() function, the
legacy s3fwrn5_nci_rf_configure() function in the same file lacks this check.

In s3fwrn5_nci_rf_configure(), if the firmware size provided by
request_firmware() is not a multiple of 4, the checksum loop:

    for (i = 0; i < fw->size; i += 4)
        checksum += *((u32 *)(fw->data+i));
        
will perform an out-of-bounds read of up to 3 bytes past the allocated
buffer. If the buffer ends exactly at a page boundary, can this cause an
invalid page fault or KASAN out-of-bounds read?

Would it make sense to add a similar size validation check to the legacy
s3fwrn5_nci_rf_configure() path while you are modifying this area?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260705190621.128257-1-jorijnvdgraaf@catcrafts.net?part=2