[PATCH 0/2] pstore: fix two ramoops error-path bugs

[PATCH 0/2] pstore: fix two ramoops error-path bugs

[Andrea Parri]

Two bugs in the ramoops error paths, found by review and confirmed with
test-only reproducers against an unfixed kernel:

 - ramoops_init_prz() stores persistent_ram_new()'s ERR_PTR in the zone
   pointer and returns without clearing it; the probe error path then
   passes it to persistent_ram_free(), which dereferences it and oopses.

 - persistent_ram_buffer_map() computes buffer_size = size -
   sizeof(struct persistent_ram_buffer) with no lower bound, so a zone no
   larger than the header underflows buffer_size to a huge value and
   defeats the bounds checks in the write paths.

The second patch adds a new failure return from persistent_ram_new() for
undersized zones, which is exactly the error path that exposes the first
bug: a tiny zone now leaves an ERR_PTR behind on cleanup.  Applied
without the first patch, it turns the memory-corruption bug into a
probe-time panic on the same input, so the ERR_PTR fix is sent first.

Andrea Parri (2):
  pstore: don't leave an ERR_PTR in the ramoops zone pointer
  pstore: reject persistent RAM zones too small for the buffer header

 fs/pstore/ram.c      | 1 +
 fs/pstore/ram_core.c | 6 ++++++
 2 files changed, 7 insertions(+)

-- 
2.53.0

[PATCH 1/2] pstore: don't leave an ERR_PTR in the ramoops zone pointer

[Andrea Parri]

ramoops_init_prz() stores the result of persistent_ram_new() in *prz and
returns the error without clearing it when initialization fails.
ramoops_probe() then cleans up through ramoops_free_przs(), which hands
*prz to persistent_ram_free(); that treats the ERR_PTR as a valid zone
and dereferences it, crashing the kernel.

Clear *prz before returning so that cleanup sees a NULL zone.

Fixes: b5d38e9bf1b0c ("pstore/ram: Add console messages handling")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Andrea Parri <parri.andrea@gmail.com>
---
 fs/pstore/ram.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
index 2eb0d4fa21869..aa056d0395a4d 100644
--- a/fs/pstore/ram.c
+++ b/fs/pstore/ram.c
@@ -604,6 +604,7 @@ static int ramoops_init_prz(const char *name,
 	if (IS_ERR(*prz)) {
 		int err = PTR_ERR(*prz);
 
+		*prz = NULL;
 		dev_err(dev, "failed to request %s mem region (0x%zx@0x%llx): %d\n",
 			name, sz, (unsigned long long)*paddr, err);
 		return err;
-- 
2.53.0

[PATCH 2/2] pstore: reject persistent RAM zones too small for the buffer header

[Andrea Parri]

persistent_ram_buffer_map() computes

	prz->buffer_size = size - sizeof(struct persistent_ram_buffer);

without checking that size is large enough to hold the header.  A
ramoops zone configured with a tiny size (e.g. ramoops.pmsg_size=1)
underflows buffer_size to a huge value, which defeats every bounds
check in persistent_ram_write() and persistent_ram_write_user(); a
write to /dev/pmsg0 then copies user data far past the end of the zone
and corrupts kernel memory.

Reject zones that are not larger than the buffer header.

Fixes: 404a6043385de ("staging: android: persistent_ram: handle reserving and mapping memory")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Andrea Parri <parri.andrea@gmail.com>
---
 fs/pstore/ram_core.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/fs/pstore/ram_core.c b/fs/pstore/ram_core.c
index b4ec8009b8dc9..ae941777037b8 100644
--- a/fs/pstore/ram_core.c
+++ b/fs/pstore/ram_core.c
@@ -505,6 +505,12 @@ static int persistent_ram_buffer_map(phys_addr_t start, phys_addr_t size,
 	prz->paddr = start;
 	prz->size = size;
 
+	if (size <= sizeof(struct persistent_ram_buffer)) {
+		pr_err("%s: persistent ram zone too small (%#llx bytes)\n",
+		       prz->label, (unsigned long long)size);
+		return -EINVAL;
+	}
+
 	if (pfn_valid(start >> PAGE_SHIFT))
 		prz->vaddr = persistent_ram_vmap(start, size, memtype);
 	else
-- 
2.53.0

Re: [PATCH 2/2] pstore: reject persistent RAM zones too small for the buffer header

[Kees Cook]

On Mon, Sep 21, 2026 at 10:23:16PM +0200, Andrea Parri wrote:
> persistent_ram_buffer_map() computes
> 
> 	prz->buffer_size = size - sizeof(struct persistent_ram_buffer);
> 
> without checking that size is large enough to hold the header.  A
> ramoops zone configured with a tiny size (e.g. ramoops.pmsg_size=1)
> underflows buffer_size to a huge value, which defeats every bounds
> check in persistent_ram_write() and persistent_ram_write_user(); a
> write to /dev/pmsg0 then copies user data far past the end of the zone
> and corrupts kernel memory.
> 
> Reject zones that are not larger than the buffer header.
> 
> Fixes: 404a6043385de ("staging: android: persistent_ram: handle reserving and mapping memory")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Andrea Parri <parri.andrea@gmail.com>
> ---
>  fs/pstore/ram_core.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/fs/pstore/ram_core.c b/fs/pstore/ram_core.c
> index b4ec8009b8dc9..ae941777037b8 100644
> --- a/fs/pstore/ram_core.c
> +++ b/fs/pstore/ram_core.c
> @@ -505,6 +505,12 @@ static int persistent_ram_buffer_map(phys_addr_t start, phys_addr_t size,
>  	prz->paddr = start;
>  	prz->size = size;
>  
> +	if (size <= sizeof(struct persistent_ram_buffer)) {
> +		pr_err("%s: persistent ram zone too small (%#llx bytes)\n",
> +		       prz->label, (unsigned long long)size);
> +		return -EINVAL;
> +	}

Whoops. Luckily it's all system-owner or hardware defined (hardware
probes, and boot params).

I wonder if it should return ENOSPC instead, though?

> +
>  	if (pfn_valid(start >> PAGE_SHIFT))
>  		prz->vaddr = persistent_ram_vmap(start, size, memtype);
>  	else
> -- 
> 2.53.0
> 

Thanks for finding this!

-- 
Kees Cook

Re: [PATCH 0/2] pstore: fix two ramoops error-path bugs

[Kees Cook]

On Mon, 21 Sep 2026 22:23:14 +0200, Andrea Parri wrote:
> Two bugs in the ramoops error paths, found by review and confirmed with
> test-only reproducers against an unfixed kernel:
> 
>  - ramoops_init_prz() stores persistent_ram_new()'s ERR_PTR in the zone
>    pointer and returns without clearing it; the probe error path then
>    passes it to persistent_ram_free(), which dereferences it and oopses.
> 
> [...]

Applied to for-next/pstore, thanks!

[1/2] pstore: don't leave an ERR_PTR in the ramoops zone pointer
      https://git.kernel.org/kees/c/70fec428c845
[2/2] pstore: reject persistent RAM zones too small for the buffer header
      https://git.kernel.org/kees/c/1b30df8662ac

Take care,

-- 
Kees Cook

Re: [PATCH 2/2] pstore: reject persistent RAM zones too small for the buffer header

[Andrea Parri]

(Dropped anton.vorontsov@linaro.org and john.stultz@linaro.org from Cc,
both bounced.)

> I wonder if it should return ENOSPC instead, though?

Fair point: nothing branches on the specific errno, but ENOSPC matches
the "doesn't fit" convention used elsewhere in ram.c, while EINVAL is
otherwise reserved for malformed config values.  I'll send a one-line
patch on top of 1b30df8662ac switching it, unless you'd rather fold it
in yourself.

Thanks!

Andrea

Re: [PATCH 2/2] pstore: reject persistent RAM zones too small for the buffer header

[Kees Cook]

On Tue, Sep 22, 2026 at 12:38:24AM +0200, Andrea Parri wrote:
> (Dropped anton.vorontsov@linaro.org and john.stultz@linaro.org from Cc,
> both bounced.)

AFAICT, Anton isn't involved any more. But, hey John, you need a .mailmap
entry. :)

> > I wonder if it should return ENOSPC instead, though?
> 
> Fair point: nothing branches on the specific errno, but ENOSPC matches
> the "doesn't fit" convention used elsewhere in ram.c, while EINVAL is
> otherwise reserved for malformed config values.  I'll send a one-line
> patch on top of 1b30df8662ac switching it, unless you'd rather fold it
> in yourself.

I'm happy to fold it in. Thank you!

-Kees

-- 
Kees Cook