[PATCH v2 0/2] nvmem: fix a const-unsoundness in reg_write

[PATCH v2 0/2] nvmem: fix a const-unsoundness in reg_write

[Link Mauve]

This callback used to take a mutable void * for no reason, which causes
the compiler to be unaware that the val buffer should never be modified
by the callback.

This was found while drafting the nvmem-provider Rust abstraction.

Thanks to the guidance of Andy Shevchenko, this now introduces a new
callback and deprecates the existing one, with the goal of renaming the
new one into the old one once no user remains in the kernel.

Changes since v1:
- Link to v1:
  https://lore.kernel.org/rust-for-linux/20260715175229.24672-1-linkmauve@linkmauve.fr/
- Removed all changes to other subsystems than nvmem.
- Added a new reg_write_const callback instead of changing the exisitng
  reg_write.
- Deprecated the existing reg_write callback, it will get removed once
  all users in the kernel will be done migrating to the new one.

Link Mauve (2):
  nvmem: core: deprecate reg_write callback and add reg_write_const
  nvmem: make all reg_write callbacks take const void *

 drivers/nvmem/apple-spmi-nvmem.c    |  2 +-
 drivers/nvmem/bcm-ocotp.c           |  6 +++---
 drivers/nvmem/core.c                | 26 +++++++++++++++++---------
 drivers/nvmem/imx-ocotp-scu.c       |  6 +++---
 drivers/nvmem/imx-ocotp.c           |  6 +++---
 drivers/nvmem/internals.h           |  1 +
 drivers/nvmem/lan9662-otpc.c        |  6 +++---
 drivers/nvmem/lpc18xx_eeprom.c      |  6 +++---
 drivers/nvmem/max77759-nvmem.c      |  4 ++--
 drivers/nvmem/meson-efuse.c         |  6 +++---
 drivers/nvmem/qcom-spmi-sdam.c      |  4 ++--
 drivers/nvmem/qfprom.c              |  6 +++---
 drivers/nvmem/rave-sp-eeprom.c      |  6 +++---
 drivers/nvmem/snvs_lpgpr.c          |  4 ++--
 drivers/nvmem/sprd-efuse.c          |  6 +++---
 drivers/nvmem/stm32-bsec-optee-ta.c |  2 +-
 drivers/nvmem/stm32-bsec-optee-ta.h |  4 ++--
 drivers/nvmem/stm32-romem.c         | 10 +++++-----
 drivers/nvmem/zynqmp_nvmem.c        |  6 +++---
 include/linux/nvmem-provider.h      |  6 +++++-
 20 files changed, 68 insertions(+), 55 deletions(-)

-- 
2.55.0


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[PATCH v2 1/2] nvmem: core: deprecate reg_write callback and add reg_write_const

[Link Mauve]

This callback used to take a mutable void * for no reason, which causes
the compiler to be unaware that the val buffer should never be modified
by the callback.

This was found while drafting the nvmem-provider Rust abstraction.

Signed-off-by: Link Mauve <linkmauve@linkmauve.fr>
---
 drivers/nvmem/core.c           | 26 +++++++++++++++++---------
 drivers/nvmem/internals.h      |  1 +
 include/linux/nvmem-provider.h |  6 +++++-
 3 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index e871181751f3..24c9e6f953c6 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -62,13 +62,20 @@ static int __nvmem_reg_read(struct nvmem_device *nvmem, unsigned int offset,
 }
 
 static int __nvmem_reg_write(struct nvmem_device *nvmem, unsigned int offset,
-			     void *val, size_t bytes)
+			     const void *val, size_t bytes)
 {
 	int ret;
 
+	if (nvmem->reg_write_const) {
+		gpiod_set_value_cansleep(nvmem->wp_gpio, 0);
+		ret = nvmem->reg_write_const(nvmem->priv, offset, val, bytes);
+		gpiod_set_value_cansleep(nvmem->wp_gpio, 1);
+		return ret;
+	}
+
 	if (nvmem->reg_write) {
 		gpiod_set_value_cansleep(nvmem->wp_gpio, 0);
-		ret = nvmem->reg_write(nvmem->priv, offset, val, bytes);
+		ret = nvmem->reg_write(nvmem->priv, offset, (void *)val, bytes);
 		gpiod_set_value_cansleep(nvmem->wp_gpio, 1);
 		return ret;
 	}
@@ -264,7 +271,7 @@ static ssize_t bin_attr_nvmem_write(struct file *filp, struct kobject *kobj,
 
 	count = round_down(count, nvmem->word_size);
 
-	if (!nvmem->reg_write || nvmem->read_only)
+	if (!nvmem->reg_write || !nvmem->reg_write_const || nvmem->read_only)
 		return -EPERM;
 
 	rc = nvmem_reg_write(nvmem, pos, buf, count);
@@ -285,7 +292,7 @@ static umode_t nvmem_bin_attr_get_umode(struct nvmem_device *nvmem)
 	if (!nvmem->read_only)
 		mode |= 0200;
 
-	if (!nvmem->reg_write)
+	if (!nvmem->reg_write || !nvmem->reg_write_const)
 		mode &= ~0200;
 
 	if (!nvmem->reg_read)
@@ -321,13 +328,13 @@ static umode_t nvmem_attr_is_visible(struct kobject *kobj,
 	struct nvmem_device *nvmem = to_nvmem_device(dev);
 
 	/*
-	 * If the device has no .reg_write operation, do not allow
-	 * configuration as read-write.
+	 * If the device has no .reg_write or .reg_write_const operation, do
+	 * not allow configuration as read-write.
 	 * If the device is set as read-only by configuration, it
 	 * can be forced into read-write mode using the 'force_ro'
 	 * attribute.
 	 */
-	if (attr == &dev_attr_force_ro.attr && !nvmem->reg_write)
+	if (attr == &dev_attr_force_ro.attr && !nvmem->reg_write && !nvmem->reg_write_const)
 		return 0;	/* Attribute not visible */
 
 	return attr->mode;
@@ -905,7 +912,7 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
 	if (!config->dev)
 		return ERR_PTR(-EINVAL);
 
-	if (!config->reg_read && !config->reg_write)
+	if (!config->reg_read && !config->reg_write && !config->reg_write_const)
 		return ERR_PTR(-EINVAL);
 
 	nvmem = kzalloc_obj(*nvmem);
@@ -950,6 +957,7 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
 	nvmem->type = config->type;
 	nvmem->reg_read = config->reg_read;
 	nvmem->reg_write = config->reg_write;
+	nvmem->reg_write_const = config->reg_write_const;
 	nvmem->keepout = config->keepout;
 	nvmem->nkeepout = config->nkeepout;
 	if (config->of_node)
@@ -975,7 +983,7 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
 		goto err_put_device;
 
 	nvmem->read_only = device_property_present(config->dev, "read-only") ||
-			   config->read_only || !nvmem->reg_write;
+			   config->read_only || !nvmem->reg_write || !nvmem->reg_write_const;
 
 #ifdef CONFIG_NVMEM_SYSFS
 	nvmem->dev.groups = nvmem_dev_groups;
diff --git a/drivers/nvmem/internals.h b/drivers/nvmem/internals.h
index 18fed57270e5..184711dfd6a6 100644
--- a/drivers/nvmem/internals.h
+++ b/drivers/nvmem/internals.h
@@ -29,6 +29,7 @@ struct nvmem_device {
 	unsigned int		nkeepout;
 	nvmem_reg_read_t	reg_read;
 	nvmem_reg_write_t	reg_write;
+	nvmem_reg_write_const_t	reg_write_const;
 	struct gpio_desc	*wp_gpio;
 	struct nvmem_layout	*layout;
 	void *priv;
diff --git a/include/linux/nvmem-provider.h b/include/linux/nvmem-provider.h
index f3b13da78aac..c6e77ba1a8c1 100644
--- a/include/linux/nvmem-provider.h
+++ b/include/linux/nvmem-provider.h
@@ -20,6 +20,8 @@ typedef int (*nvmem_reg_read_t)(void *priv, unsigned int offset,
 				void *val, size_t bytes);
 typedef int (*nvmem_reg_write_t)(void *priv, unsigned int offset,
 				 void *val, size_t bytes);
+typedef int (*nvmem_reg_write_const_t)(void *priv, unsigned int offset,
+				       const void *val, size_t bytes);
 /* used for vendor specific post processing of cell data */
 typedef int (*nvmem_cell_post_process_t)(void *priv, const char *id, int index,
 					 unsigned int offset, void *buf,
@@ -93,7 +95,8 @@ struct nvmem_cell_info {
  * @root_only:	Device is accessibly to root only.
  * @of_node:	If given, this will be used instead of the parent's of_node.
  * @reg_read:	Callback to read data; return zero if successful.
- * @reg_write:	Callback to write data; return zero if successful.
+ * @reg_write:	**DEPRECATED** - please use reg_write_const instead.
+ * @reg_write_const:	Callback to write data; return zero if successful.
  * @size:	Device size.
  * @word_size:	Minimum read/write access granularity.
  * @stride:	Minimum read/write access stride.
@@ -128,6 +131,7 @@ struct nvmem_config {
 	struct device_node	*of_node;
 	nvmem_reg_read_t	reg_read;
 	nvmem_reg_write_t	reg_write;
+	nvmem_reg_write_const_t	reg_write_const;
 	int	size;
 	int	word_size;
 	int	stride;
-- 
2.55.0


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH v2 1/2] nvmem: core: deprecate reg_write callback and add reg_write_const

[sashiko-bot]

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

New issues:
- [High] Incorrect boolean logic using '||' instead of '&&' forces all NVMEM devices to be treated as read-only and unconditionally disables hardware writes.

Pre-existing issues:
- [High] Concurrent nvmem writes race on the write-protect GPIO, which can lead to unprotected hardware states and failed writes.
--

--- Patch [1]: [PATCH v2 1/2] nvmem: core: deprecate reg_write callback and add reg_write_const ---
commit fd9b6bd1e67a30d018b7e8f0d9db60403988e05a
Author: Link Mauve <linkmauve@linkmauve.fr>

nvmem: core: deprecate reg_write callback and add reg_write_const

This patch deprecates the reg_write callback in favor of reg_write_const,
which takes a const pointer for the data buffer. It updates the nvmem core to
support either callback when checking for write capabilities.

> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
> index 9d182eda0c994..66a85d31149d7 100644
> --- a/drivers/nvmem/core.c
> +++ b/drivers/nvmem/core.c
> @@ -62,13 +62,20 @@ static int __nvmem_reg_read(struct nvmem_device *nvmem, unsigned int offset,
>  }
>  
>  static int __nvmem_reg_write(struct nvmem_device *nvmem, unsigned int offset,
> -			     void *val, size_t bytes)
> +			     const void *val, size_t bytes)
>  {
>  	int ret;
>  
> +	if (nvmem->reg_write_const) {
> +		gpiod_set_value_cansleep(nvmem->wp_gpio, 0);
> +		ret = nvmem->reg_write_const(nvmem->priv, offset, val, bytes);
> +		gpiod_set_value_cansleep(nvmem->wp_gpio, 1);
> +		return ret;
> +	}
> +

[Severity: High]
This is a pre-existing issue, but does toggling the wp_gpio here have a race
condition?

If two threads concurrently write to the same NVMEM device, they could
execute __nvmem_reg_write() simultaneously. One thread could re-assert the
write-protect GPIO while the other thread's hardware write is still in
progress, potentially causing the write to fail or corrupt the write sequence.

>  	if (nvmem->reg_write) {
>  		gpiod_set_value_cansleep(nvmem->wp_gpio, 0);
> -		ret = nvmem->reg_write(nvmem->priv, offset, val, bytes);
> +		ret = nvmem->reg_write(nvmem->priv, offset, (void *)val, bytes);
>  		gpiod_set_value_cansleep(nvmem->wp_gpio, 1);
>  		return ret;
>  	}

[ ... ]

> @@ -264,7 +271,7 @@ static ssize_t bin_attr_nvmem_write(struct file *filp, struct kobject *kobj,
>  
>  	count = round_down(count, nvmem->word_size);
>  
> -	if (!nvmem->reg_write || nvmem->read_only)
> +	if (!nvmem->reg_write || !nvmem->reg_write_const || nvmem->read_only)
>  		return -EPERM;

[Severity: High]
Does this logic inadvertently reject all writes?

Since drivers will typically provide either reg_write or reg_write_const
(but not both), this condition will always evaluate to true, failing all
sysfs writes with -EPERM. Should this be && instead of ||?

[ ... ]

> @@ -285,7 +292,7 @@ static umode_t nvmem_bin_attr_get_umode(struct nvmem_device *nvmem)
>  	if (!nvmem->read_only)
>  		mode |= 0200;
>  
> -	if (!nvmem->reg_write)
> +	if (!nvmem->reg_write || !nvmem->reg_write_const)
>  		mode &= ~0200;

[Severity: High]
Similar to the sysfs write function above, does this strip write permissions
from sysfs attributes for all devices?

Since at least one of these callbacks will typically be NULL, this condition
is always true.

[ ... ]

> @@ -975,7 +983,7 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
>  		goto err_put_device;
>  
>  	nvmem->read_only = device_property_present(config->dev, "read-only") ||
> -			   config->read_only || !nvmem->reg_write;
> +			   config->read_only || !nvmem->reg_write || !nvmem->reg_write_const;

[Severity: High]
Does this unconditionally set the device to read-only during registration?

Using || means any device that doesn't provide both callbacks will be marked
as read-only.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260715195520.25410-1-linkmauve@linkmauve.fr?part=1

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[PATCH v2 2/2] nvmem: make all reg_write callbacks take const void *

[Link Mauve]

The previous commit switched from a pointer to mutable data to a pointer
to immutable data, so let’s fix all users of this API.

Signed-off-by: Link Mauve <linkmauve@linkmauve.fr>
---
 drivers/nvmem/apple-spmi-nvmem.c    |  2 +-
 drivers/nvmem/bcm-ocotp.c           |  6 +++---
 drivers/nvmem/imx-ocotp-scu.c       |  6 +++---
 drivers/nvmem/imx-ocotp.c           |  6 +++---
 drivers/nvmem/lan9662-otpc.c        |  6 +++---
 drivers/nvmem/lpc18xx_eeprom.c      |  6 +++---
 drivers/nvmem/max77759-nvmem.c      |  4 ++--
 drivers/nvmem/meson-efuse.c         |  6 +++---
 drivers/nvmem/qcom-spmi-sdam.c      |  4 ++--
 drivers/nvmem/qfprom.c              |  6 +++---
 drivers/nvmem/rave-sp-eeprom.c      |  6 +++---
 drivers/nvmem/snvs_lpgpr.c          |  4 ++--
 drivers/nvmem/sprd-efuse.c          |  6 +++---
 drivers/nvmem/stm32-bsec-optee-ta.c |  2 +-
 drivers/nvmem/stm32-bsec-optee-ta.h |  4 ++--
 drivers/nvmem/stm32-romem.c         | 10 +++++-----
 drivers/nvmem/zynqmp_nvmem.c        |  6 +++---
 17 files changed, 45 insertions(+), 45 deletions(-)

diff --git a/drivers/nvmem/apple-spmi-nvmem.c b/drivers/nvmem/apple-spmi-nvmem.c
index 88614005d5ce..cbf25c53d048 100644
--- a/drivers/nvmem/apple-spmi-nvmem.c
+++ b/drivers/nvmem/apple-spmi-nvmem.c
@@ -29,7 +29,7 @@ static int apple_spmi_nvmem_probe(struct spmi_device *sdev)
 		.stride = 1,
 		.size = 0xffff,
 		.reg_read = (void *)regmap_bulk_read,
-		.reg_write = (void *)regmap_bulk_write,
+		.reg_write_const = (void *)regmap_bulk_write,
 	};
 
 	regmap = devm_regmap_init_spmi_ext(sdev, &apple_spmi_regmap_config);
diff --git a/drivers/nvmem/bcm-ocotp.c b/drivers/nvmem/bcm-ocotp.c
index 2490f44caa40..5853ae0fe02d 100644
--- a/drivers/nvmem/bcm-ocotp.c
+++ b/drivers/nvmem/bcm-ocotp.c
@@ -179,11 +179,11 @@ static int bcm_otpc_read(void *context, unsigned int offset, void *val,
 	return 0;
 }
 
-static int bcm_otpc_write(void *context, unsigned int offset, void *val,
+static int bcm_otpc_write(void *context, unsigned int offset, const void *val,
 	size_t bytes)
 {
 	struct otpc_priv *priv = context;
-	u32 *buf = val;
+	const u32 *buf = val;
 	u32 bytes_written;
 	u32 address = offset / priv->config->word_size;
 	int i, ret;
@@ -223,7 +223,7 @@ static struct nvmem_config bcm_otpc_nvmem_config = {
 	.word_size = 4,
 	.stride = 4,
 	.reg_read = bcm_otpc_read,
-	.reg_write = bcm_otpc_write,
+	.reg_write_const = bcm_otpc_write,
 };
 
 static const struct of_device_id bcm_otpc_dt_ids[] = {
diff --git a/drivers/nvmem/imx-ocotp-scu.c b/drivers/nvmem/imx-ocotp-scu.c
index 517d83e11af2..57008228100f 100644
--- a/drivers/nvmem/imx-ocotp-scu.c
+++ b/drivers/nvmem/imx-ocotp-scu.c
@@ -178,11 +178,11 @@ static int imx_scu_ocotp_read(void *context, unsigned int offset,
 }
 
 static int imx_scu_ocotp_write(void *context, unsigned int offset,
-			       void *val, size_t bytes)
+			       const void *val, size_t bytes)
 {
 	struct ocotp_priv *priv = context;
 	struct arm_smccc_res res;
-	u32 *buf = val;
+	const u32 *buf = val;
 	u32 tmp;
 	u32 index;
 	int ret;
@@ -226,7 +226,7 @@ static struct nvmem_config imx_scu_ocotp_nvmem_config = {
 	.stride = 1,
 	.owner = THIS_MODULE,
 	.reg_read = imx_scu_ocotp_read,
-	.reg_write = imx_scu_ocotp_write,
+	.reg_write_const = imx_scu_ocotp_write,
 };
 
 static const struct of_device_id imx_scu_ocotp_dt_ids[] = {
diff --git a/drivers/nvmem/imx-ocotp.c b/drivers/nvmem/imx-ocotp.c
index 108d78d7f6cb..36d4a37aed93 100644
--- a/drivers/nvmem/imx-ocotp.c
+++ b/drivers/nvmem/imx-ocotp.c
@@ -310,11 +310,11 @@ static void imx_ocotp_set_imx7_timing(struct ocotp_priv *priv)
 	writel(timing, priv->base + IMX_OCOTP_ADDR_TIMING);
 }
 
-static int imx_ocotp_write(void *context, unsigned int offset, void *val,
+static int imx_ocotp_write(void *context, unsigned int offset, const void *val,
 			   size_t bytes)
 {
 	struct ocotp_priv *priv = context;
-	u32 *buf = val;
+	const u32 *buf = val;
 	int ret;
 
 	u32 ctrl;
@@ -483,7 +483,7 @@ static struct nvmem_config imx_ocotp_nvmem_config = {
 	.word_size = 4,
 	.stride = 1,
 	.reg_read = imx_ocotp_read,
-	.reg_write = imx_ocotp_write,
+	.reg_write_const = imx_ocotp_write,
 };
 
 static const struct ocotp_params imx6q_params = {
diff --git a/drivers/nvmem/lan9662-otpc.c b/drivers/nvmem/lan9662-otpc.c
index 56fc19f092a7..7e4579c404d4 100644
--- a/drivers/nvmem/lan9662-otpc.c
+++ b/drivers/nvmem/lan9662-otpc.c
@@ -140,10 +140,10 @@ static int lan9662_otp_read(void *context, unsigned int offset,
 }
 
 static int lan9662_otp_write(void *context, unsigned int offset,
-			     void *_val, size_t bytes)
+			     const void *_val, size_t bytes)
 {
 	struct lan9662_otp *otp = context;
-	u8 *val = _val;
+	const u8 *val = _val;
 	u8 data, newdata;
 	int i, rc = 0;
 
@@ -175,7 +175,7 @@ static struct nvmem_config otp_config = {
 	.stride = 1,
 	.word_size = 1,
 	.reg_read = lan9662_otp_read,
-	.reg_write = lan9662_otp_write,
+	.reg_write_const = lan9662_otp_write,
 	.size = OTP_MEM_SIZE,
 };
 
diff --git a/drivers/nvmem/lpc18xx_eeprom.c b/drivers/nvmem/lpc18xx_eeprom.c
index 504155e30bab..78a35ef095fb 100644
--- a/drivers/nvmem/lpc18xx_eeprom.c
+++ b/drivers/nvmem/lpc18xx_eeprom.c
@@ -87,7 +87,7 @@ static int lpc18xx_eeprom_busywait_until_prog(struct lpc18xx_eeprom_dev *eeprom)
 }
 
 static int lpc18xx_eeprom_gather_write(void *context, unsigned int reg,
-				       void *val, size_t bytes)
+				       const void *val, size_t bytes)
 {
 	struct lpc18xx_eeprom_dev *eeprom = context;
 	unsigned int offset = reg;
@@ -109,7 +109,7 @@ static int lpc18xx_eeprom_gather_write(void *context, unsigned int reg,
 	usleep_range(100, 200);
 
 	while (bytes) {
-		writel(*(u32 *)val, eeprom->mem_base + offset);
+		writel(*(const u32 *)val, eeprom->mem_base + offset);
 		ret = lpc18xx_eeprom_busywait_until_prog(eeprom);
 		if (ret < 0)
 			return ret;
@@ -155,7 +155,7 @@ static struct nvmem_config lpc18xx_nvmem_config = {
 	.stride = 4,
 	.word_size = 4,
 	.reg_read = lpc18xx_eeprom_read,
-	.reg_write = lpc18xx_eeprom_gather_write,
+	.reg_write_const = lpc18xx_eeprom_gather_write,
 };
 
 static int lpc18xx_eeprom_probe(struct platform_device *pdev)
diff --git a/drivers/nvmem/max77759-nvmem.c b/drivers/nvmem/max77759-nvmem.c
index 283000ec3a2c..86c025cfc2f6 100644
--- a/drivers/nvmem/max77759-nvmem.c
+++ b/drivers/nvmem/max77759-nvmem.c
@@ -59,7 +59,7 @@ static int max77759_nvmem_reg_read(void *priv, unsigned int offset,
 }
 
 static int max77759_nvmem_reg_write(void *priv, unsigned int offset,
-				    void *val, size_t bytes)
+				    const void *val, size_t bytes)
 {
 	struct max77759_nvmem *nvmem = priv;
 	DEFINE_FLEX(struct max77759_maxq_command, cmd, cmd, length,
@@ -99,7 +99,7 @@ static int max77759_nvmem_probe(struct platform_device *pdev)
 		.word_size = sizeof(u8),
 		.stride = sizeof(u8),
 		.reg_read = max77759_nvmem_reg_read,
-		.reg_write = max77759_nvmem_reg_write,
+		.reg_write_const = max77759_nvmem_reg_write,
 	};
 	struct max77759_nvmem *nvmem;
 
diff --git a/drivers/nvmem/meson-efuse.c b/drivers/nvmem/meson-efuse.c
index d7f9ac99a212..94b24d11a1a3 100644
--- a/drivers/nvmem/meson-efuse.c
+++ b/drivers/nvmem/meson-efuse.c
@@ -27,12 +27,12 @@ static int meson_efuse_read(void *context, unsigned int offset,
 }
 
 static int meson_efuse_write(void *context, unsigned int offset,
-			     void *val, size_t bytes)
+			     const void *val, size_t bytes)
 {
 	struct meson_sm_firmware *fw = context;
 	int ret;
 
-	ret = meson_sm_call_write(fw, (u8 *)val, bytes, SM_EFUSE_WRITE, offset,
+	ret = meson_sm_call_write(fw, (void *)val, bytes, SM_EFUSE_WRITE, offset,
 				  bytes, 0, 0, 0);
 
 	return ret < 0 ? ret : 0;
@@ -83,7 +83,7 @@ static int meson_efuse_probe(struct platform_device *pdev)
 	econfig->stride = 1;
 	econfig->word_size = 1;
 	econfig->reg_read = meson_efuse_read;
-	econfig->reg_write = meson_efuse_write;
+	econfig->reg_write_const = meson_efuse_write;
 	econfig->size = size;
 	econfig->priv = fw;
 
diff --git a/drivers/nvmem/qcom-spmi-sdam.c b/drivers/nvmem/qcom-spmi-sdam.c
index 4f1cca6eab71..6758cbc34577 100644
--- a/drivers/nvmem/qcom-spmi-sdam.c
+++ b/drivers/nvmem/qcom-spmi-sdam.c
@@ -81,7 +81,7 @@ static int sdam_read(void *priv, unsigned int offset, void *val,
 	return rc;
 }
 
-static int sdam_write(void *priv, unsigned int offset, void *val,
+static int sdam_write(void *priv, unsigned int offset, const void *val,
 				size_t bytes)
 {
 	struct sdam_chip *sdam = priv;
@@ -147,7 +147,7 @@ static int sdam_probe(struct platform_device *pdev)
 	sdam->sdam_config.size = sdam->size;
 	sdam->sdam_config.word_size = 1;
 	sdam->sdam_config.reg_read = sdam_read;
-	sdam->sdam_config.reg_write = sdam_write;
+	sdam->sdam_config.reg_write_const = sdam_write;
 	sdam->sdam_config.priv = sdam;
 
 	nvmem = devm_nvmem_register(&pdev->dev, &sdam->sdam_config);
diff --git a/drivers/nvmem/qfprom.c b/drivers/nvmem/qfprom.c
index 1de3435df116..16b7f647a3f6 100644
--- a/drivers/nvmem/qfprom.c
+++ b/drivers/nvmem/qfprom.c
@@ -252,13 +252,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, const void *_val,
 			    size_t bytes)
 {
 	struct qfprom_priv *priv = context;
 	struct qfprom_touched_values old;
 	int words = bytes / 4;
-	u32 *value = _val;
+	const u32 *value = _val;
 	u32 blow_status;
 	int ret;
 	int i;
@@ -443,7 +443,7 @@ static int qfprom_probe(struct platform_device *pdev)
 
 		/* Only enable writing if we have SoC data and a valid clock */
 		if (priv->soc_data && priv->secclk)
-			econfig.reg_write = qfprom_reg_write;
+			econfig.reg_write_const = qfprom_reg_write;
 	}
 
 	pm_runtime_enable(dev);
diff --git a/drivers/nvmem/rave-sp-eeprom.c b/drivers/nvmem/rave-sp-eeprom.c
index 9ecf3873cbb7..c703d303eb6c 100644
--- a/drivers/nvmem/rave-sp-eeprom.c
+++ b/drivers/nvmem/rave-sp-eeprom.c
@@ -278,10 +278,10 @@ static int rave_sp_eeprom_reg_read(void *eeprom, unsigned int offset,
 }
 
 static int rave_sp_eeprom_reg_write(void *eeprom, unsigned int offset,
-				    void *val, size_t bytes)
+				    const void *val, size_t bytes)
 {
 	return rave_sp_eeprom_access(eeprom, RAVE_SP_EEPROM_WRITE,
-				     offset, val, bytes);
+				     offset, (void *)val, bytes);
 }
 
 static int rave_sp_eeprom_probe(struct platform_device *pdev)
@@ -331,7 +331,7 @@ static int rave_sp_eeprom_probe(struct platform_device *pdev)
 	config.add_legacy_fixed_of_cells	= true;
 	config.size		= size;
 	config.reg_read		= rave_sp_eeprom_reg_read;
-	config.reg_write	= rave_sp_eeprom_reg_write;
+	config.reg_write_const	= rave_sp_eeprom_reg_write;
 	config.word_size	= 1;
 	config.stride		= 1;
 
diff --git a/drivers/nvmem/snvs_lpgpr.c b/drivers/nvmem/snvs_lpgpr.c
index 89c27112320f..3e76d33dff20 100644
--- a/drivers/nvmem/snvs_lpgpr.c
+++ b/drivers/nvmem/snvs_lpgpr.c
@@ -50,7 +50,7 @@ static const struct snvs_lpgpr_cfg snvs_lpgpr_cfg_imx7d = {
 	.size		= 16,
 };
 
-static int snvs_lpgpr_write(void *context, unsigned int offset, void *val,
+static int snvs_lpgpr_write(void *context, unsigned int offset, const void *val,
 			    size_t bytes)
 {
 	struct snvs_lpgpr_priv *priv = context;
@@ -127,7 +127,7 @@ static int snvs_lpgpr_probe(struct platform_device *pdev)
 	cfg->size = dcfg->size;
 	cfg->owner = THIS_MODULE;
 	cfg->reg_read  = snvs_lpgpr_read;
-	cfg->reg_write = snvs_lpgpr_write;
+	cfg->reg_write_const = snvs_lpgpr_write;
 
 	nvmem = devm_nvmem_register(dev, cfg);
 
diff --git a/drivers/nvmem/sprd-efuse.c b/drivers/nvmem/sprd-efuse.c
index 1a7e4e5d8b86..f960fa8cb25c 100644
--- a/drivers/nvmem/sprd-efuse.c
+++ b/drivers/nvmem/sprd-efuse.c
@@ -192,7 +192,7 @@ static void sprd_efuse_set_prog_en(struct sprd_efuse *efuse, bool en)
 }
 
 static int sprd_efuse_raw_prog(struct sprd_efuse *efuse, u32 blk, bool doub,
-			       bool lock, u32 *data)
+			       bool lock, const u32 *data)
 {
 	u32 status;
 	int ret = 0;
@@ -321,7 +321,7 @@ static int sprd_efuse_read(void *context, u32 offset, void *val, size_t bytes)
 	return ret;
 }
 
-static int sprd_efuse_write(void *context, u32 offset, void *val, size_t bytes)
+static int sprd_efuse_write(void *context, u32 offset, const void *val, size_t bytes)
 {
 	struct sprd_efuse *efuse = context;
 	bool blk_double = efuse->data->blk_double;
@@ -410,7 +410,7 @@ static int sprd_efuse_probe(struct platform_device *pdev)
 	econfig.size = efuse->data->blk_nums * SPRD_EFUSE_BLOCK_WIDTH;
 	econfig.add_legacy_fixed_of_cells = true;
 	econfig.reg_read = sprd_efuse_read;
-	econfig.reg_write = sprd_efuse_write;
+	econfig.reg_write_const = sprd_efuse_write;
 	econfig.priv = efuse;
 	econfig.dev = &pdev->dev;
 	nvmem = devm_nvmem_register(&pdev->dev, &econfig);
diff --git a/drivers/nvmem/stm32-bsec-optee-ta.c b/drivers/nvmem/stm32-bsec-optee-ta.c
index f89ce791dd12..3184b976b1f9 100644
--- a/drivers/nvmem/stm32-bsec-optee-ta.c
+++ b/drivers/nvmem/stm32-bsec-optee-ta.c
@@ -203,7 +203,7 @@ int stm32_bsec_optee_ta_read(struct tee_context *ctx, unsigned int offset,
 
 /* stm32_bsec_optee_ta_write() - nvmem write access using PTA client driver */
 int stm32_bsec_optee_ta_write(struct tee_context *ctx, unsigned int lower,
-			      unsigned int offset, void *buf, size_t bytes)
+			      unsigned int offset, const void *buf, size_t bytes)
 {	struct tee_shm *shm;
 	struct tee_ioctl_invoke_arg arg;
 	struct tee_param param[2];
diff --git a/drivers/nvmem/stm32-bsec-optee-ta.h b/drivers/nvmem/stm32-bsec-optee-ta.h
index 3966a0535179..74658a54fe57 100644
--- a/drivers/nvmem/stm32-bsec-optee-ta.h
+++ b/drivers/nvmem/stm32-bsec-optee-ta.h
@@ -50,7 +50,7 @@ int stm32_bsec_optee_ta_read(struct tee_context *ctx, unsigned int offset,
  *	On success, 0. On failure, -errno.
  */
 int stm32_bsec_optee_ta_write(struct tee_context *ctx, unsigned int lower,
-			      unsigned int offset, void *buf, size_t bytes);
+			      unsigned int offset, const void *buf, size_t bytes);
 
 #else
 
@@ -72,7 +72,7 @@ static inline int stm32_bsec_optee_ta_read(struct tee_context *ctx,
 
 static inline int stm32_bsec_optee_ta_write(struct tee_context *ctx,
 					    unsigned int lower,
-					    unsigned int offset, void *buf,
+					    unsigned int offset, const void *buf,
 					    size_t bytes)
 {
 	return -EOPNOTSUPP;
diff --git a/drivers/nvmem/stm32-romem.c b/drivers/nvmem/stm32-romem.c
index 82879b1c9eb9..622093b0755c 100644
--- a/drivers/nvmem/stm32-romem.c
+++ b/drivers/nvmem/stm32-romem.c
@@ -118,12 +118,12 @@ static int stm32_bsec_read(void *context, unsigned int offset, void *buf,
 	return 0;
 }
 
-static int stm32_bsec_write(void *context, unsigned int offset, void *buf,
+static int stm32_bsec_write(void *context, unsigned int offset, const void *buf,
 			    size_t bytes)
 {
 	struct stm32_romem_priv *priv = context;
 	struct device *dev = priv->cfg.dev;
-	u32 *buf32 = buf;
+	const u32 *buf32 = buf;
 	int ret, i;
 
 	/* Allow only writing complete 32-bits aligned words */
@@ -153,7 +153,7 @@ static int stm32_bsec_pta_read(void *context, unsigned int offset, void *buf,
 	return stm32_bsec_optee_ta_read(priv->ctx, offset, buf, bytes);
 }
 
-static int stm32_bsec_pta_write(void *context, unsigned int offset, void *buf,
+static int stm32_bsec_pta_write(void *context, unsigned int offset, const void *buf,
 				size_t bytes)
 {
 	struct stm32_romem_priv *priv = context;
@@ -239,10 +239,10 @@ static int stm32_romem_probe(struct platform_device *pdev)
 				return rc;
 			}
 			priv->cfg.reg_read = stm32_bsec_pta_read;
-			priv->cfg.reg_write = stm32_bsec_pta_write;
+			priv->cfg.reg_write_const = stm32_bsec_pta_write;
 		} else {
 			priv->cfg.reg_read = stm32_bsec_read;
-			priv->cfg.reg_write = stm32_bsec_write;
+			priv->cfg.reg_write_const = stm32_bsec_write;
 		}
 	}
 
diff --git a/drivers/nvmem/zynqmp_nvmem.c b/drivers/nvmem/zynqmp_nvmem.c
index d297ff150dc0..54905a608157 100644
--- a/drivers/nvmem/zynqmp_nvmem.c
+++ b/drivers/nvmem/zynqmp_nvmem.c
@@ -183,7 +183,7 @@ static int zynqmp_nvmem_read(void *context, unsigned int offset, void *val, size
 }
 
 static int zynqmp_nvmem_write(void *context,
-			      unsigned int offset, void *val, size_t bytes)
+			      unsigned int offset, const void *val, size_t bytes)
 {
 	int pufflag = 0;
 
@@ -194,7 +194,7 @@ static int zynqmp_nvmem_write(void *context,
 		pufflag = 1;
 
 	return zynqmp_efuse_access(context, offset,
-				   val, bytes, EFUSE_WRITE, pufflag);
+				   (void *)val, bytes, EFUSE_WRITE, pufflag);
 }
 
 static const struct of_device_id zynqmp_nvmem_match[] = {
@@ -216,7 +216,7 @@ static int zynqmp_nvmem_probe(struct platform_device *pdev)
 	econfig.priv = dev;
 	econfig.add_legacy_fixed_of_cells = true;
 	econfig.reg_read = zynqmp_nvmem_read;
-	econfig.reg_write = zynqmp_nvmem_write;
+	econfig.reg_write_const = zynqmp_nvmem_write;
 
 	return PTR_ERR_OR_ZERO(devm_nvmem_register(dev, &econfig));
 }
-- 
2.55.0


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH v2 2/2] nvmem: make all reg_write callbacks take const void *

[sashiko-bot]

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

New issues:
- [Critical] The nvmem core incorrectly evaluates write permissions, causing all drivers providing only `.reg_write_const` to be unconditionally treated as read-only.

Pre-existing issues:
- [Critical] The Spreadtrum eFuse driver incorrectly calculates memory offsets and size constraints, leading to out-of-bounds MMIO writes, memory corruption, and kernel stack information leaks.
- [High] The bcm-ocotp driver fails to disable programming mode on the error path.
- [High] The stm32-bsec-optee-ta driver leaks TEE sessions when unaligned writes are requested.
- [High] The imx-ocotp-scu driver has a Time-of-Check to Time-of-Use (TOCTOU) race condition when writing to ECC regions.
- [High] The meson-sm firmware driver exposes a lockless shared memory race when interacting with meson-efuse.
- [High] The lpc18xx-eeprom driver lacks serialization, causing a race condition on the EEPROM power state that can crash the system.
- [High] The qfprom driver modifies global clock rates and regulator voltages without serialization.
- [High] The lan9662-otpc driver modifies hardware state and registers without locking, leading to race conditions and fuse corruption.
- [High] The bcm-ocotp driver modifies command and address registers without locking, leading to OTP fuse corruption.
--

--- Patch [2]: [PATCH v2 2/2] nvmem: make all reg_write callbacks take const void * ---
commit 757b2f38ab10e7e5415fbd52779fa12f901c3ac4
Author: Link Mauve <linkmauve@linkmauve.fr>
Subject: nvmem: 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 this API.

> diff --git a/drivers/nvmem/apple-spmi-nvmem.c b/drivers/nvmem/apple-spmi-nvmem.c
> --- a/drivers/nvmem/apple-spmi-nvmem.c
> +++ b/drivers/nvmem/apple-spmi-nvmem.c
> @@ -29,7 +29,7 @@ static int apple_spmi_nvmem_probe(struct spmi_device *sdev)
>  		.stride = 1,
>  		.size = 0xffff,
>  		.reg_read = (void *)regmap_bulk_read,
> -		.reg_write = (void *)regmap_bulk_write,
> +		.reg_write_const = (void *)regmap_bulk_write,

[Severity: Critical]
By migrating to .reg_write_const and leaving .reg_write as NULL, does this
inadvertently make all these devices read-only?

Looking at the nvmem core in drivers/nvmem/core.c:

nvmem_register() {
    ...
    nvmem->read_only = device_property_present(config->dev, "read-only") ||
                       config->read_only || !nvmem->reg_write ||
                       !nvmem->reg_write_const;
    ...
}

bin_attr_nvmem_write() {
    ...
    if (!nvmem->reg_write || !nvmem->reg_write_const || nvmem->read_only)
        return -EPERM;
    ...
}

Because the core checks !nvmem->reg_write || !nvmem->reg_write_const,
setting reg_write to NULL causes the core to strip write permissions and
return -EPERM for all sysfs writes.

> diff --git a/drivers/nvmem/bcm-ocotp.c b/drivers/nvmem/bcm-ocotp.c
> --- a/drivers/nvmem/bcm-ocotp.c
> +++ b/drivers/nvmem/bcm-ocotp.c
> [ ... ]
> @@ -179,11 +179,11 @@ static int bcm_otpc_read(void *context, unsigned int offset, void *val,
>  	return 0;
>  }
>  
> -static int bcm_otpc_write(void *context, unsigned int offset, void *val,
> +static int bcm_otpc_write(void *context, unsigned int offset, const void *val,
>  	size_t bytes)
>  {
>  	struct otpc_priv *priv = context;
> -	u32 *buf = val;
> +	const u32 *buf = val;

[Severity: High]
This isn't a bug introduced by this patch, but is it possible for concurrent
reads and writes to corrupt hardware state?

In bcm_otpc_write():
    for (bytes_written = 0; bytes_written < bytes;) {
        set_command(priv->base, OTPC_CMD_PROGRAM);
        set_cpu_address(priv->base, address++);

And similarly in bcm_otpc_read():
    for (bytes_read = 0; bytes_read < bytes;) {
        set_command(priv->base, OTPC_CMD_READ);
        set_cpu_address(priv->base, address++);

Without serialization, concurrent sysfs operations could cause the command and
address registers to be overwritten mid-operation by another thread,
potentially redirecting writes to unintended fuse locations.

[Severity: High]
This isn't a bug introduced by this patch, but does the error path in
bcm_otpc_write() permanently leave the hardware in programming mode?

    ret = enable_ocotp_program(priv->base);
    if (ret)
        return -EIO;

    for (bytes_written = 0; bytes_written < bytes;) {
        ...
        ret = poll_cpu_status(priv->base, OTPC_STAT_CMD_DONE);
        reset_start_bit(priv->base);
        if (ret) {
            dev_err(priv->dev, "otp write error: 0x%x", ret);
            return -EIO;
        }
    }

If poll_cpu_status() returns an error, the function returns -EIO directly and
skips the disable_ocotp_program(priv->base) call at the end of the function,
potentially leaving programming voltages applied.

> diff --git a/drivers/nvmem/imx-ocotp-scu.c b/drivers/nvmem/imx-ocotp-scu.c
> --- a/drivers/nvmem/imx-ocotp-scu.c
> +++ b/drivers/nvmem/imx-ocotp-scu.c
> @@ -178,11 +178,11 @@ static int imx_scu_ocotp_read(void *context, unsigned int offset,
>  }
>  
>  static int imx_scu_ocotp_write(void *context, unsigned int offset,
> -			       void *val, size_t bytes)
> +			       const void *val, size_t bytes)
>  {
>  	struct ocotp_priv *priv = context;
>  	struct arm_smccc_res res;
> -	u32 *buf = val;
> +	const u32 *buf = val;

[Severity: High]
This isn't a bug introduced by this patch, but is there a race condition when
writing to ECC regions in imx_scu_ocotp_write()?

    if (in_ecc(context, index)) {
        pr_warn("ECC region, only program once\n");
        mutex_lock(&scu_ocotp_mutex);
        ret = imx_sc_misc_otp_fuse_read(priv->nvmem_ipc, index, &tmp);
        mutex_unlock(&scu_ocotp_mutex);
        if (ret)
            return ret;
        if (tmp) {
            pr_warn("ECC region, already has value: %x\n", tmp);
            return -EIO;
        }
    }

    mutex_lock(&scu_ocotp_mutex);
    arm_smccc_smc(IMX_SIP_OTP_WRITE, index, *buf, 0, 0, 0, 0, 0, &res);
    mutex_unlock(&scu_ocotp_mutex);

Because the lock is released after checking if the ECC region is programmed and
then re-acquired for the SMC write, two concurrent threads could both pass the
initial check and perform a double-write to the write-once ECC region.

> diff --git a/drivers/nvmem/lan9662-otpc.c b/drivers/nvmem/lan9662-otpc.c
> --- a/drivers/nvmem/lan9662-otpc.c
> +++ b/drivers/nvmem/lan9662-otpc.c
> @@ -140,10 +140,10 @@ static int lan9662_otp_read(void *context, unsigned int offset,
>  }
>  
>  static int lan9662_otp_write(void *context, unsigned int offset,
> -			     void *_val, size_t bytes)
> +			     const void *_val, size_t bytes)
>  {
>  	struct lan9662_otp *otp = context;
> -	u8 *val = _val;
> +	const u8 *val = _val;

[Severity: High]
This isn't a bug introduced by this patch, but can concurrent accesses to
lan9662_otp_write() and lan9662_otp_read() corrupt OTP operations?

    for (i = 0; i < bytes; i++) {
        if (val[i]) {
            rc = lan9662_otp_read_byte(otp, offset + i, &data);
            if (rc < 0)
                break;
            ...
            rc = lan9662_otp_write_byte(otp, offset + i, newdata);

Since these hardware register accesses and state changes (such as powering up
and polling) are performed without holding a local lock, concurrent sysfs
operations might overwrite each other's commands in flight, potentially
corrupting OTP data.

> diff --git a/drivers/nvmem/lpc18xx_eeprom.c b/drivers/nvmem/lpc18xx_eeprom.c
> --- a/drivers/nvmem/lpc18xx_eeprom.c
> +++ b/drivers/nvmem/lpc18xx_eeprom.c
> @@ -88,7 +88,7 @@ static int lpc18xx_eeprom_busywait_until_prog(struct lpc18xx_eeprom_dev *eeprom)
>  }
>  
>  static int lpc18xx_eeprom_gather_write(void *context, unsigned int reg,
> -				       void *val, size_t bytes)
> +				       const void *val, size_t bytes)
>  {
>  	struct lpc18xx_eeprom_dev *eeprom = context;
>  	unsigned int offset = reg;
> @@ -110,7 +110,7 @@ static int lpc18xx_eeprom_gather_write(void *context, unsigned int reg,
>  	usleep_range(100, 200);
>  
>  	while (bytes) {
> -		writel(*(u32 *)val, eeprom->mem_base + offset);
> +		writel(*(const u32 *)val, eeprom->mem_base + offset);

[Severity: High]
This isn't a bug introduced by this patch, but does the lack of a lock around
EEPROM power management in lpc18xx_eeprom_gather_write() and
lpc18xx_eeprom_read() risk a system crash?

    lpc18xx_eeprom_writel(eeprom, LPC18XX_EEPROM_PWRDWN,
                          LPC18XX_EEPROM_PWRDWN_NO);
    usleep_range(100, 200);
    ...
    writel(*(const u32 *)val, eeprom->mem_base + offset);

    lpc18xx_eeprom_writel(eeprom, LPC18XX_EEPROM_PWRDWN,
                          LPC18XX_EEPROM_PWRDWN_YES);

If a read and write occur concurrently, one thread might power down the EEPROM
while the other is still inside its memory access loop. Accessing powered-down
MMIO regions typically causes a synchronous data abort.

> diff --git a/drivers/nvmem/meson-efuse.c b/drivers/nvmem/meson-efuse.c
> --- a/drivers/nvmem/meson-efuse.c
> +++ b/drivers/nvmem/meson-efuse.c
> @@ -27,12 +27,12 @@ static int meson_efuse_read(void *context, unsigned int offset,
>  }
>  
>  static int meson_efuse_write(void *context, unsigned int offset,
> -			     void *val, size_t bytes)
> +			     const void *val, size_t bytes)
>  {
>  	struct meson_sm_firmware *fw = context;
>  	int ret;
>  
> -	ret = meson_sm_call_write(fw, (u8 *)val, bytes, SM_EFUSE_WRITE, offset,
> +	ret = meson_sm_call_write(fw, (void *)val, bytes, SM_EFUSE_WRITE, offset,

[Severity: High]
This isn't a bug introduced by this patch, but does meson_sm_call_write()
expose a lockless shared memory race?

In drivers/firmware/meson/meson_sm.c:
    memcpy(fw->sm_shmem_in_base, buffer, size);
    if (meson_sm_call(fw, cmd_index, &written, arg0, arg1, arg2, arg3, arg4) < 0)
        return -EINVAL;

Since fw->sm_shmem_in_base is a global shared memory buffer and there is no
locking, concurrent writes could overwrite each other's payloads before the SMC
call executes, potentially writing corrupted or arbitrary data to the
write-once eFuses.

> diff --git a/drivers/nvmem/qfprom.c b/drivers/nvmem/qfprom.c
> --- 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, const void *_val,
>  			    size_t bytes)
>  {
>  	struct qfprom_priv *priv = context;
>  	struct qfprom_touched_values old;
>  	int words = bytes / 4;
> -	u32 *value = _val;
> +	const u32 *value = _val;

[Severity: High]
This isn't a bug introduced by this patch, but can concurrent calls to
qfprom_reg_write() leave the system clock and regulators permanently unbalanced?

    ret = qfprom_enable_fuse_blowing(priv, &old);
    ...
    qfprom_disable_fuse_blowing(priv, &old);

Inside qfprom_enable_fuse_blowing():
    old->clk_rate = clk_get_rate(priv->secclk);
    ret = clk_set_rate(priv->secclk, priv->soc_data->qfprom_blow_set_freq);

Because the old state is saved to a local stack variable without any mutex
serialization, a concurrent write could save the already-elevated clock rate
and regulator voltage as the "old" state. Upon completion, the hardware would
be restored to this elevated state rather than the original one.

> diff --git a/drivers/nvmem/sprd-efuse.c b/drivers/nvmem/sprd-efuse.c
> --- a/drivers/nvmem/sprd-efuse.c
> +++ b/drivers/nvmem/sprd-efuse.c
> @@ -192,7 +192,7 @@ static void sprd_efuse_set_prog_en(struct sprd_efuse *efuse, bool en)
>  }
>  
>  static int sprd_efuse_raw_prog(struct sprd_efuse *efuse, u32 blk, bool doub,
> -			       bool lock, u32 *data)
> +			       bool lock, const u32 *data)
>  {
>  	u32 status;
>  	int ret = 0;
> [ ... ]
> @@ -321,7 +321,7 @@ static int sprd_efuse_read(void *context, u32 offset, void *val, size_t bytes)
>  	return ret;
>  }
>  
> -static int sprd_efuse_write(void *context, u32 offset, void *val, size_t bytes)
> +static int sprd_efuse_write(void *context, u32 offset, const void *val, size_t bytes)
>  {
>  	struct sprd_efuse *efuse = context;
>  	bool blk_double = efuse->data->blk_double;

[Severity: Critical]
This isn't a bug introduced by this patch, but does the offset handling in
sprd_efuse_write() and sprd_efuse_read() lead to out-of-bounds access?

In sprd_efuse_write(), the raw byte offset is passed directly as the block
index to sprd_efuse_raw_prog():

    ret = sprd_efuse_raw_prog(efuse, offset, blk_double, lock, val);

Then, in sprd_efuse_raw_prog():

    writel(*data, efuse->base + SPRD_EFUSE_MEM(blk));

The macro SPRD_EFUSE_MEM(blk) shifts the offset by 2 (multiplying by 4).
Because word_size = 1 and stride = 1, userspace can request arbitrary offsets,
causing this to write far outside the intended eFuse MMIO region.

Additionally, in sprd_efuse_read():

    u32 data;
    ...
    ret = sprd_efuse_raw_read(efuse, index, &data, blk_double);
    if (!ret) {
        data >>= blk_offset;
        memcpy(val, &data, bytes);
    }

If userspace requests a read larger than 4 bytes, memcpy() will read past the
local stack variable 'data' and copy uninitialized adjacent kernel stack memory
to userspace.

> diff --git a/drivers/nvmem/stm32-bsec-optee-ta.c b/drivers/nvmem/stm32-bsec-optee-ta.c
> --- a/drivers/nvmem/stm32-bsec-optee-ta.c
> +++ b/drivers/nvmem/stm32-bsec-optee-ta.c
> @@ -203,7 +203,7 @@ int stm32_bsec_optee_ta_read(struct tee_context *ctx, unsigned int offset,
>  
>  /* stm32_bsec_optee_ta_write() - nvmem write access using PTA client driver */
>  int stm32_bsec_optee_ta_write(struct tee_context *ctx, unsigned int lower,
> -			      unsigned int offset, void *buf, size_t bytes)
> +			      unsigned int offset, const void *buf, size_t bytes)
>  {	struct tee_shm *shm;

[Severity: High]
This isn't a bug introduced by this patch, but does an unaligned write request
leak TEE sessions?

    ret = stm32_bsec_ta_open_session(ctx, &session_id);
    if (ret)
        return ret;

    /* Allow only writing complete 32-bits aligned words */
    if ((bytes % 4) || (offset % 4))
        return -EINVAL;

If the offset or bytes are not 32-bit aligned, the function returns -EINVAL
directly, bypassing the call to stm32_bsec_ta_close_session(ctx, session_id).
Because the nvmem core allows unaligned writes by default, this could be
triggered repeatedly to exhaust all available TEE sessions.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260715195520.25410-1-linkmauve@linkmauve.fr?part=2

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic