[PATCH v4 0/2] nvmem-reboot-mode: support small reboot mode magic

[PATCH v4 0/2] nvmem-reboot-mode: support small reboot mode magic

[Jennifer Berringer]

Hi all,

These patches allow nvmem-reboot-mode to handle devices with a
reboot-mode cell smaller than 4 bytes, such as Qualcomm SA8775P. Without
this change, nvmem_reboot_mode_write() tries to write 4 bytes to a
1-byte nvmem cell and fails with the message "update reboot mode bits
failed" on affected devices.

This issue previously resulted in an out-of-bounds heap write but, after
one of my patches from v3 of this series was accepted, it results in
that error message instead.

[1]
https://lore.kernel.org/all/20241104152312.3813601-2-jberring@redhat.com/

v4:
- Excluded one patch which was accepted from v3 [1]:
	"nvmem: core: improve range check for nvmem_cell_write()"
- Handle truncation of the reboot mode magic in nvmem-reboot-mode
  instead of in nvmem core.
- Introduced nvmem_cell_size().

v3:
- Accepted suggestion for changing nvmem_cell_write bounds check

v2:
- Added missing function definition for CONFIG_NVMEM=n

Jennifer Berringer (2):
  nvmem: core: add nvmem_cell_size()
  power: reset: nvmem-reboot-mode: support smaller magic

 drivers/nvmem/core.c                    | 18 ++++++++++++++++++
 drivers/power/reset/nvmem-reboot-mode.c |  9 ++++++++-
 include/linux/nvmem-consumer.h          |  6 ++++++
 3 files changed, 32 insertions(+), 1 deletion(-)


base-commit: ffd294d346d185b70e28b1a28abe367bbfe53c04
-- 
2.47.1

[PATCH v4 1/2] nvmem: core: add nvmem_cell_size()

[Jennifer Berringer]

This function allows nvmem consumers to know the size of an nvmem cell
before calling nvmem_cell_write() or nvmem_cell_read(), which is helpful
for drivers that may need to handle devices with different cell sizes.

Signed-off-by: Jennifer Berringer <jberring@redhat.com>
---
 drivers/nvmem/core.c           | 18 ++++++++++++++++++
 include/linux/nvmem-consumer.h |  6 ++++++
 2 files changed, 24 insertions(+)

diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index d6494dfc20a7..4d0cbd20da48 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -1624,6 +1624,24 @@ void nvmem_cell_put(struct nvmem_cell *cell)
 }
 EXPORT_SYMBOL_GPL(nvmem_cell_put);
 
+/**
+ * nvmem_cell_size() - Get nvmem cell size in bytes.
+ *
+ * @cell: nvmem cell.
+ *
+ * Return: size of the nvmem cell.
+ */
+size_t nvmem_cell_size(struct nvmem_cell *cell)
+{
+	struct nvmem_cell_entry *entry = cell->entry;
+
+	if (!entry)
+		return 0;
+
+	return entry->bytes;
+}
+EXPORT_SYMBOL_GPL(nvmem_cell_size);
+
 static void nvmem_shift_read_buffer_in_place(struct nvmem_cell_entry *cell, void *buf)
 {
 	u8 *p, *b;
diff --git a/include/linux/nvmem-consumer.h b/include/linux/nvmem-consumer.h
index 34c0e58dfa26..a2020527d2d3 100644
--- a/include/linux/nvmem-consumer.h
+++ b/include/linux/nvmem-consumer.h
@@ -54,6 +54,7 @@ struct nvmem_cell *nvmem_cell_get(struct device *dev, const char *id);
 struct nvmem_cell *devm_nvmem_cell_get(struct device *dev, const char *id);
 void nvmem_cell_put(struct nvmem_cell *cell);
 void devm_nvmem_cell_put(struct device *dev, struct nvmem_cell *cell);
+size_t nvmem_cell_size(struct nvmem_cell *cell);
 void *nvmem_cell_read(struct nvmem_cell *cell, size_t *len);
 int nvmem_cell_write(struct nvmem_cell *cell, void *buf, size_t len);
 int nvmem_cell_read_u8(struct device *dev, const char *cell_id, u8 *val);
@@ -117,6 +118,11 @@ static inline void nvmem_cell_put(struct nvmem_cell *cell)
 {
 }
 
+static inline size_t nvmem_cell_size(struct nvmem_cell *cell)
+{
+	return 0;
+}
+
 static inline void *nvmem_cell_read(struct nvmem_cell *cell, size_t *len)
 {
 	return ERR_PTR(-EOPNOTSUPP);
-- 
2.47.1

Re: [PATCH v4 1/2] nvmem: core: add nvmem_cell_size()

[Srinivas Kandagatla]

Hi Jennifer,

On 28/02/2025 18:03, Jennifer Berringer wrote:
> This function allows nvmem consumers to know the size of an nvmem cell
> before calling nvmem_cell_write() or nvmem_cell_read(), which is helpful
> for drivers that may need to handle devices with different cell sizes.
> 
> Signed-off-by: Jennifer Berringer <jberring@redhat.com>
> ---
>   drivers/nvmem/core.c           | 18 ++++++++++++++++++
>   include/linux/nvmem-consumer.h |  6 ++++++
>   2 files changed, 24 insertions(+)
> 
> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
> index d6494dfc20a7..4d0cbd20da48 100644
> --- a/drivers/nvmem/core.c
> +++ b/drivers/nvmem/core.c
> @@ -1624,6 +1624,24 @@ void nvmem_cell_put(struct nvmem_cell *cell)
>   }
>   EXPORT_SYMBOL_GPL(nvmem_cell_put);
>   
> +/**
> + * nvmem_cell_size() - Get nvmem cell size in bytes.
> + *
> + * @cell: nvmem cell.
> + *
> + * Return: size of the nvmem cell.
> + */
> +size_t nvmem_cell_size(struct nvmem_cell *cell)
> +{
> +	struct nvmem_cell_entry *entry = cell->entry;
> +
> +	if (!entry)
> +		return 0;
> +
> +	return entry->bytes;
> +}
> +EXPORT_SYMBOL_GPL(nvmem_cell_size);
> +

There is a similar patch on the list, could you take a look and see if 
that is usable in power reset driver.

https://lore.kernel.org/lkml/20250306093900.2199442-3-o.rempel@pengutronix.de/T/#m97bbb1870d7140661894a4e806a695e563588524



--srini
>   static void nvmem_shift_read_buffer_in_place(struct nvmem_cell_entry *cell, void *buf)
>   {
>   	u8 *p, *b;
> diff --git a/include/linux/nvmem-consumer.h b/include/linux/nvmem-consumer.h
> index 34c0e58dfa26..a2020527d2d3 100644
> --- a/include/linux/nvmem-consumer.h
> +++ b/include/linux/nvmem-consumer.h
> @@ -54,6 +54,7 @@ struct nvmem_cell *nvmem_cell_get(struct device *dev, const char *id);
>   struct nvmem_cell *devm_nvmem_cell_get(struct device *dev, const char *id);
>   void nvmem_cell_put(struct nvmem_cell *cell);
>   void devm_nvmem_cell_put(struct device *dev, struct nvmem_cell *cell);
> +size_t nvmem_cell_size(struct nvmem_cell *cell);
>   void *nvmem_cell_read(struct nvmem_cell *cell, size_t *len);
>   int nvmem_cell_write(struct nvmem_cell *cell, void *buf, size_t len);
>   int nvmem_cell_read_u8(struct device *dev, const char *cell_id, u8 *val);
> @@ -117,6 +118,11 @@ static inline void nvmem_cell_put(struct nvmem_cell *cell)
>   {
>   }
>   
> +static inline size_t nvmem_cell_size(struct nvmem_cell *cell)
> +{
> +	return 0;
> +}
> +
>   static inline void *nvmem_cell_read(struct nvmem_cell *cell, size_t *len)
>   {
>   	return ERR_PTR(-EOPNOTSUPP);

Re: [PATCH v4 1/2] nvmem: core: add nvmem_cell_size()

[Jennifer Berringer]

Hello,

On 3/7/25 12:42, Srinivas Kandagatla wrote:
> Hi Jennifer,
> 
> There is a similar patch on the list, could you take a look and see if that is usable in power reset driver.
> 
> https://lore.kernel.org/lkml/20250306093900.2199442-3-o.rempel@pengutronix.de/T/#m97bbb1870d7140661894a4e806a695e563588524
> 
> --srini

Yes, the nvmem_cell_get_size() function defined in that patch would make my own nvmem_cell_size() function in this patch redundant. I can update my reboot mode driver change to depend on that patch instead of introducing my own function for the same purpose.

[PATCH v4 2/2] power: reset: nvmem-reboot-mode: support smaller magic

[Jennifer Berringer]

Some devices, such as Qualcomm sa8775p, have an nvmem reboot mode cell
that is only 1 byte, which resulted in nvmem_reboot_mode_write() failing
when it attempts to write a 4-byte magic. Checking the nvmem cell size
and writing only the lower bits of the reboot mode magic is needed for
these devices.

Signed-off-by: Jennifer Berringer <jberring@redhat.com>
---
 drivers/power/reset/nvmem-reboot-mode.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/power/reset/nvmem-reboot-mode.c b/drivers/power/reset/nvmem-reboot-mode.c
index 41530b70cfc4..112e6cf77d1b 100644
--- a/drivers/power/reset/nvmem-reboot-mode.c
+++ b/drivers/power/reset/nvmem-reboot-mode.c
@@ -20,11 +20,18 @@ static int nvmem_reboot_mode_write(struct reboot_mode_driver *reboot,
 				    unsigned int magic)
 {
 	int ret;
+	u8 *magic_ptr = (u8 *) &magic;
+	size_t cell_size;
 	struct nvmem_reboot_mode *nvmem_rbm;
 
 	nvmem_rbm = container_of(reboot, struct nvmem_reboot_mode, reboot);
+	cell_size = nvmem_cell_size(nvmem_rbm->cell);
 
-	ret = nvmem_cell_write(nvmem_rbm->cell, &magic, sizeof(magic));
+	/* Use magic's low-order bytes when writing to a smaller cell. */
+	if (IS_ENABLED(CONFIG_CPU_BIG_ENDIAN) && cell_size < sizeof(magic))
+		magic_ptr += sizeof(magic) - cell_size;
+
+	ret = nvmem_cell_write(nvmem_rbm->cell, magic_ptr, MIN(cell_size, sizeof(magic)));
 	if (ret < 0)
 		dev_err(reboot->dev, "update reboot mode bits failed\n");
 
-- 
2.47.1