Re: [PATCH v5 1/6] nfc: s3fwrn5: drop of_match_ptr() from the I2C driver

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

Pre-existing issues:
- [High] Preexisting endianness bug in S3FWRN5 firmware packet length parsing and construction, leading to wrong length values on big-endian architectures.
--

--- Patch [1]: [PATCH v5 1/6] nfc: s3fwrn5: drop of_match_ptr() from the I2C driver ---
commit 5cffdb84b592ca672d62f53d2f736890c7d61164
Author: Jorijn van der Graaf <[email protected]>

nfc: s3fwrn5: drop of_match_ptr() from the I2C driver

This commit drops of_match_ptr() from the I2C driver to ensure the same
code is used across all configurations. The OF match table now relies on
MODULE_DEVICE_TABLE() to count as used.

> diff --git a/drivers/nfc/s3fwrn5/i2c.c b/drivers/nfc/s3fwrn5/i2c.c
> index 4ba7626117118..cb73993446e09 100644
> --- a/drivers/nfc/s3fwrn5/i2c.c
> +++ b/drivers/nfc/s3fwrn5/i2c.c

[ ... ]

> @@ -219,7 +219,7 @@ MODULE_DEVICE_TABLE(of, of_s3fwrn5_i2c_match);
>  static struct i2c_driver s3fwrn5_i2c_driver = {
>  	.driver = {
>  		.name = S3FWRN5_I2C_DRIVER_NAME,
> -		.of_match_table = of_match_ptr(of_s3fwrn5_i2c_match),
> +		.of_match_table = of_s3fwrn5_i2c_match,
>  	},
>  	.probe = s3fwrn5_i2c_probe,
>  	.remove = s3fwrn5_i2c_remove,

[Severity: High]
This is a pre-existing issue, but does s3fwrn5_i2c_read() parse the firmware
header length correctly on big-endian systems?

Earlier in drivers/nfc/s3fwrn5/i2c.c:s3fwrn5_i2c_read():

	data_len = (phy->common.mode == S3FWRN5_MODE_NCI) ?
		((struct nci_ctrl_hdr *)hdr)->plen :
		((struct s3fwrn5_fw_header *)hdr)->len;

Because struct s3fwrn5_fw_header defines len as a __u16, reading it directly
from the packet without endianness conversion like le16_to_cpu() swaps the
bytes on big-endian architectures. 

Can this lead to allocating an incorrectly sized skb and breaking firmware
communication on these systems?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1