Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Missing endianness conversions when parsing firmware headers and offsets, leading to failures on Big Endian architectures.
--
--- Patch [2]: [PATCH v4 2/6] nfc: s3fwrn5: use the "s3fwrn5_i2c" string literal directly ---
commit 9d4d91e92081c012d8837ad241bfc3be79ee322a
Author: Jorijn van der Graaf [off-list ref]
nfc: s3fwrn5: use the "s3fwrn5_i2c" string literal directly
This commit replaces the S3FWRN5_I2C_DRIVER_NAME define with the string
literal "s3fwrn5_i2c" directly in the driver. The define was an indirection
for a fixed ABI name and was only used locally within this file.
quoted hunk ↗ jump to hunk
diff --git a/drivers/nfc/s3fwrn5/i2c.c b/drivers/nfc/s3fwrn5/i2c.c
index cb73993446e09..c528c085b43ef 100644
--- a/drivers/nfc/s3fwrn5/i2c.c
+++ b/drivers/nfc/s3fwrn5/i2c.c
[ ... ]
[Severity: High]
This is a pre-existing issue, but can this code cause failures on Big Endian
architectures due to missing endianness conversions?
In s3fwrn5_i2c_read(), the 16-bit length field is read directly in native
byte order:
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;
...
}
Could this improperly byte-swap on Big Endian systems, leading to oversized
allocations in alloc_skb() and invalid I2C reads?
Additionally, in s3fwrn5_fw_request_firmware(), 32-bit offsets are copied
directly from the little-endian firmware payload into native u32 variables
without using le32_to_cpu():
drivers/nfc/s3fwrn5/firmware.c:s3fwrn5_fw_request_firmware() {
...
memcpy(&sig_off, fw->fw->data + 0x14, 4);
fw->sig = fw->fw->data + sig_off;
...
}
Since sig_off is byte-swapped to a massive value on Big Endian systems, does
this result in an out-of-bounds pointer calculation?
When s3fwrn5_fw_enter_update_mode() later calls s3fwrn5_fw_prep_msg(), could
copying from this fw->sig pointer trigger an unmapped memory read and
kernel panic?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260807021001.239389-1-jorijnvdgraaf@catcrafts.net?part=2