Re: [PATCH v4 2/6] nfc: s3fwrn5: use the "s3fwrn5_i2c" string literal directly
| Newsgroups | org.kernel.vger.linux-devicetree |
|---|---|
| Message-ID | <[email protected]> |
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 <[email protected]> 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. > 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/[email protected]?part=2