Re: [PATCH v4 2/6] nfc: s3fwrn5: use the "s3fwrn5_i2c" string literal directly

From
sashiko-bot@kernel.org
Date
2026-08-08 02:24:23
Reply to
Jorijn van der Graaf, 2026-08-07
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
Thread (15 messages) 15 messages, 3 authors, last reply 2026-08-11
Keyboard shortcuts
hback out one level
jnext message in thread / next series-diff position
kprevious message in thread / previous series-diff position
ldrill in
Escclose help / fold thread tree
?toggle this help