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

[email protected]
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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.