Re: [PATCH 5/8] iio: pressure: bmp280: make reg_write callback take const void *

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Low] Syntax error due to 'void void *_val' parameter in qfprom_reg_write.

Pre-existing issues:
- [Critical] m48t59_nvram_read and m48t59_nvram_write completely ignore the requested offset.
--

--- Patch [5]: [PATCH 5/8] iio: pressure: bmp280: make reg_write callback take const void * ---
commit bd8404c8c6d692d9a700578455111f78fd0eaf2a
Author: Link Mauve <[email protected]>

iio: pressure: bmp280: make reg_write callback 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/nvmem/qfprom.c b/drivers/nvmem/qfprom.c
> index a872c640b8c5a..bacdcb5610bae 100644
> --- a/drivers/nvmem/qfprom.c
> +++ b/drivers/nvmem/qfprom.c
> @@ -253,13 +253,13 @@ static int qfprom_enable_fuse_blowing(const struct qfprom_priv *priv,
>   *
>   * Return: 0 or -err.
>   */
> -static int qfprom_reg_write(void *context, unsigned int reg, void *_val,
> +static int qfprom_reg_write(void *context, unsigned int reg, void void *_val,
>  			    size_t bytes)
>  {

[Severity: Low]
Does this syntax error in qfprom_reg_write() prevent the qfprom driver from
compiling?

It looks like "void void *_val" was introduced during a find-and-replace
to make the callback take a const void pointer.

>  	struct qfprom_priv *priv = context;
>  	struct qfprom_touched_values old;
>  	int words = bytes / 4;
> -	u32 *value = _val;
> +	const u32 *value = _val;
>  	u32 blow_status;

[ ... ]

> diff --git a/drivers/rtc/rtc-m48t59.c b/drivers/rtc/rtc-m48t59.c
> index 4e608bc8bbd36..ad407faecf1f8 100644
> --- a/drivers/rtc/rtc-m48t59.c
> +++ b/drivers/rtc/rtc-m48t59.c
> @@ -325,7 +325,7 @@ static int m48t59_nvram_write(void *priv, unsigned int offset, void *val,
>  	struct m48t59_private *m48t59 = platform_get_drvdata(pdev);
>  	ssize_t cnt = 0;
>  	unsigned long flags;
> -	u8 *buf = val;
> +	const u8 *buf = val;
>  
>  	spin_lock_irqsave(&m48t59->lock, flags);
>  

[Severity: Critical]
This is a pre-existing issue, but does the loop in m48t59_nvram_write()
completely ignore the requested offset?

It appears that cnt starts at 0 and is used as the hardware address instead
of adding the requested offset:

drivers/rtc/rtc-m48t59.c:m48t59_nvram_write() {
    ...
	for (; cnt < size; cnt++)
		M48T59_WRITE(*buf++, cnt);
    ...
}

Any subsystem or user-space application performing an NVMEM write at an
offset greater than 0 will silently overwrite data at the very beginning
of the NVRAM.

The same issue seems to be present in m48t59_nvram_read(), which could
lead to incorrect data disclosure:

drivers/rtc/rtc-m48t59.c:m48t59_nvram_read() {
    ...
	for (; cnt < size; cnt++)
		*buf++ = M48T59_READ(cnt);
    ...
}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5

_______________________________________________
linux-amlogic mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-amlogic