[4/8] misc: make all reg_write callbacks take const void *
Commit Message
The previous commit switched from a pointer to mutable data to a pointer
to immutable data, so let’s fix all users of the nvmem_config API.
Signed-off-by: Link Mauve <linkmauve@linkmauve.fr>
---
drivers/misc/ds1682.c | 2 +-
drivers/misc/eeprom/at24.c | 4 ++--
drivers/misc/eeprom/at25.c | 2 +-
drivers/misc/eeprom/eeprom_93xx46.c | 4 ++--
drivers/misc/eeprom/m24lr.c | 2 +-
drivers/misc/keba/cp500.c | 2 +-
drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_otpe2p.c | 8 ++++----
7 files changed, 12 insertions(+), 12 deletions(-)
Comments
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
New issues:
- [Low] Passing `const void *` to `nvmem_device_write` discards the `const` qualifier, causing a compiler warning/error.
Pre-existing issues:
- [Critical] The `m24lr_write` function uses the device offset to index the source memory buffer, causing a critical out-of-bounds read of kernel memory.
- [High] Writing a dereferenced `const char *` byte to a 32-bit memory-mapped register triggers sign-extension, potentially writing corrupted values to the hardware.
- [High] Silent data loss when writing an odd number of bytes in 16-bit mode.
--
--- Patch [4]: [PATCH 4/8] misc: make all reg_write callbacks take const void * ---
commit 8fae9381bce13733c89619440fc083ff647125a4
Author: Link Mauve <linkmauve@linkmauve.fr>
misc: make all reg_write callbacks take const void *
The previous commit switched from a pointer to mutable data to a pointer
to immutable data, so let's fix all users of the nvmem_config API.
[ ... ]
> diff --git a/drivers/misc/eeprom/eeprom_93xx46.c b/drivers/misc/eeprom/eeprom_93xx46.c
> index 5230e910a1d11..8de85433d593c 100644
> --- a/drivers/misc/eeprom/eeprom_93xx46.c
> +++ b/drivers/misc/eeprom/eeprom_93xx46.c
> @@ -269,10 +269,10 @@ eeprom_93xx46_write_word(struct eeprom_93xx46_dev *edev,
> }
>
> static int eeprom_93xx46_write(void *priv, unsigned int off,
> - void *val, size_t count)
> + const void *val, size_t count)
> {
> struct eeprom_93xx46_dev *edev = priv;
> - char *buf = val;
> + const char *buf = val;
[Severity: High]
This is a pre-existing issue, but does this function silently drop data when
writing an odd number of bytes in 16-bit mode?
In 16-bit word mode (EE_ADDR16), the driver intercepts the call and truncates
the count:
count &= ~1;
If count is 1, it becomes 0, skipping the write loop entirely and returning 0.
Because the nvmem core interprets a 0 return value as success, the caller
believes the data was written even though the hardware was unmodified.
[ ... ]
> diff --git a/drivers/misc/eeprom/m24lr.c b/drivers/misc/eeprom/m24lr.c
> index 7a9fd45a8e460..22f3221e232b5 100644
> --- a/drivers/misc/eeprom/m24lr.c
> +++ b/drivers/misc/eeprom/m24lr.c
> @@ -374,7 +374,7 @@ static int m24lr_nvmem_read(void *priv, unsigned int offset, void *val,
> return 0;
> }
>
> -static int m24lr_nvmem_write(void *priv, unsigned int offset, void *val,
> +static int m24lr_nvmem_write(void *priv, unsigned int offset, const void *val,
> size_t bytes)
[Severity: Critical]
This is a pre-existing issue, but does m24lr_write() use the device offset to
index the source memory buffer?
When m24lr_nvmem_write() calls m24lr_write(..., val, bytes, offset, true),
m24lr_write() passes buf + offset to the regmap write instead of advancing
by the number of bytes written:
err = m24lr_regmap_write(regmap, buf + offset, n, offset);
Since buf is only sized for the data chunk being written, adding the device
offset pushes the pointer far past the end of buf, leading to an out-of-bounds
read of kernel memory that gets written to the external EEPROM.
> {
> ssize_t err;
[ ... ]
> diff --git a/drivers/misc/keba/cp500.c b/drivers/misc/keba/cp500.c
> index 6c65fbf22e756..ee7889cb63a8b 100644
> --- a/drivers/misc/keba/cp500.c
> +++ b/drivers/misc/keba/cp500.c
> @@ -590,7 +590,7 @@ static int cp500_nvmem_read(void *priv, unsigned int offset, void *val,
> return 0;
> }
>
> -static int cp500_nvmem_write(void *priv, unsigned int offset, void *val,
> +static int cp500_nvmem_write(void *priv, unsigned int offset, const void *val,
> size_t bytes)
> {
> struct cp500_nvmem *nvmem = priv;
[Severity: Low]
Does passing the now-const val directly to nvmem_device_write() discard the
const qualifier?
nvmem_device_write() expects a mutable void pointer, which will cause a build
warning (and potentially break the build if -Werror is enabled):
ret = nvmem_device_write(nvmem->base_nvmem, nvmem->offset + offset,
bytes, val);
[ ... ]
> diff --git a/drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_otpe2p.c b/drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_otpe2p.c
> index a2ed477e0370b..f2759e9b41386 100644
> --- a/drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_otpe2p.c
> +++ b/drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_otpe2p.c
> @@ -159,11 +159,11 @@ static int pci1xxxx_eeprom_read(void *priv_t, unsigned int off,
> }
>
> static int pci1xxxx_eeprom_write(void *priv_t, unsigned int off,
> - void *value_t, size_t count)
> + const void *value_t, size_t count)
> {
> struct pci1xxxx_otp_eeprom_device *priv = priv_t;
> void __iomem *rb = priv->reg_base;
> - char *value = value_t;
> + const char *value = value_t;
[Severity: High]
This is a pre-existing issue, but does dereferencing this const char pointer
trigger sign-extension when passed to writel()?
Later in this function, the byte is written like this:
writel(*(value + byte), rb + MMAP_EEPROM_OFFSET(EEPROM_DATA_REG));
If the byte has its most significant bit set (e.g. is negative), the C compiler
will sign-extend it to a 32-bit negative integer before passing it to writel(),
which writes 1s into the upper 24 bits of the register. Should value be cast to
const u8 * to ensure safe zero-extension?
> u32 regval;
> u32 byte;
> int ret;
> @@ -262,11 +262,11 @@ static int pci1xxxx_otp_read(void *priv_t, unsigned int off,
> }
>
> static int pci1xxxx_otp_write(void *priv_t, unsigned int off,
> - void *value_t, size_t count)
> + const void *value_t, size_t count)
> {
> struct pci1xxxx_otp_eeprom_device *priv = priv_t;
> void __iomem *rb = priv->reg_base;
> - char *value = value_t;
> + const char *value = value_t;
[Severity: High]
This is a pre-existing issue, but this has the same sign-extension behavior
when writing to OTP_PRGM_DATA_OFFSET:
writel(*(value + byte), rb + MMAP_OTP_OFFSET(OTP_PRGM_DATA_OFFSET));
Should this pointer also be changed to const u8 *?
> u32 regval;
> u32 byte;
> int ret;
@@ -209,7 +209,7 @@ static int ds1682_nvmem_read(void *priv, unsigned int offset, void *val,
return ret < 0 ? ret : 0;
}
-static int ds1682_nvmem_write(void *priv, unsigned int offset, void *val,
+static int ds1682_nvmem_write(void *priv, unsigned int offset, const void *val,
size_t bytes)
{
struct i2c_client *client = priv;
@@ -477,11 +477,11 @@ static int at24_read(void *priv, unsigned int off, void *val, size_t count)
return 0;
}
-static int at24_write(void *priv, unsigned int off, void *val, size_t count)
+static int at24_write(void *priv, unsigned int off, const void *val, size_t count)
{
struct at24_data *at24;
struct device *dev;
- char *buf = val;
+ const char *buf = val;
int ret;
at24 = priv;
@@ -235,7 +235,7 @@ static int at25_wait_ready(struct at25_data *at25)
return bounce[0];
}
-static int at25_ee_write(void *priv, unsigned int off, void *val, size_t count)
+static int at25_ee_write(void *priv, unsigned int off, const void *val, size_t count)
{
u8 *bounce __free(kfree) = kmalloc(min(count, io_limit), GFP_KERNEL);
struct at25_data *at25 = priv;
@@ -268,10 +268,10 @@ eeprom_93xx46_write_word(struct eeprom_93xx46_dev *edev,
}
static int eeprom_93xx46_write(void *priv, unsigned int off,
- void *val, size_t count)
+ const void *val, size_t count)
{
struct eeprom_93xx46_dev *edev = priv;
- char *buf = val;
+ const char *buf = val;
int ret, step = 1;
unsigned int i;
@@ -374,7 +374,7 @@ static int m24lr_nvmem_read(void *priv, unsigned int offset, void *val,
return 0;
}
-static int m24lr_nvmem_write(void *priv, unsigned int offset, void *val,
+static int m24lr_nvmem_write(void *priv, unsigned int offset, const void *val,
size_t bytes)
{
ssize_t err;
@@ -590,7 +590,7 @@ static int cp500_nvmem_read(void *priv, unsigned int offset, void *val,
return 0;
}
-static int cp500_nvmem_write(void *priv, unsigned int offset, void *val,
+static int cp500_nvmem_write(void *priv, unsigned int offset, const void *val,
size_t bytes)
{
struct cp500_nvmem *nvmem = priv;
@@ -159,11 +159,11 @@ static int pci1xxxx_eeprom_read(void *priv_t, unsigned int off,
}
static int pci1xxxx_eeprom_write(void *priv_t, unsigned int off,
- void *value_t, size_t count)
+ const void *value_t, size_t count)
{
struct pci1xxxx_otp_eeprom_device *priv = priv_t;
void __iomem *rb = priv->reg_base;
- char *value = value_t;
+ const char *value = value_t;
u32 regval;
u32 byte;
int ret;
@@ -262,11 +262,11 @@ static int pci1xxxx_otp_read(void *priv_t, unsigned int off,
}
static int pci1xxxx_otp_write(void *priv_t, unsigned int off,
- void *value_t, size_t count)
+ const void *value_t, size_t count)
{
struct pci1xxxx_otp_eeprom_device *priv = priv_t;
void __iomem *rb = priv->reg_base;
- char *value = value_t;
+ const char *value = value_t;
u32 regval;
u32 byte;
int ret;