[PATCH v4.4.z] pstore/ram: Write new dumps to start of recycled zones

[PATCH v4.4.z] pstore/ram: Write new dumps to start of recycled zones

[Kees Cook]

From: Aleksandr Yashkin <a.yashkin@inango-systems.com>

[ Upstream commit 9e5f1c19800b808a37fb9815a26d382132c26c3d ]

The ram_core.c routines treat przs as circular buffers. When writing a
new crash dump, the old buffer needs to be cleared so that the new dump
doesn't end up in the wrong place (i.e. at the end).

The solution to this problem is to reset the circular buffer state before
writing a new Oops dump.

Signed-off-by: Aleksandr Yashkin <a.yashkin@inango-systems.com>
Signed-off-by: Nikolay Merinov <n.merinov@inango-systems.com>
Signed-off-by: Ariel Gilman <a.gilman@inango-systems.com>
Link: https://lore.kernel.org/r/20191223133816.28155-1-n.merinov@inango-systems.com
Fixes: 896fc1f0c4c6 ("pstore/ram: Switch to persistent_ram routines")
[kees: backport to v4.9]
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 fs/pstore/ram.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
index 59d93acc29c7..fa0e89edb62d 100644
--- a/fs/pstore/ram.c
+++ b/fs/pstore/ram.c
@@ -319,6 +319,17 @@ static int notrace ramoops_pstore_write_buf(enum pstore_type_id type,
 
 	prz = cxt->przs[cxt->dump_write_cnt];
 
+	/*
+	 * Since this is a new crash dump, we need to reset the buffer in
+	 * case it still has an old dump present. Without this, the new dump
+	 * will get appended, which would seriously confuse anything trying
+	 * to check dump file contents. Specifically, ramoops_read_kmsg_hdr()
+	 * expects to find a dump header in the beginning of buffer data, so
+	 * we must to reset the buffer values, in order to ensure that the
+	 * header will be written to the beginning of the buffer.
+	 */
+	persistent_ram_zap(prz);
+
 	hlen = ramoops_write_kmsg_hdr(prz, compressed);
 	if (size + hlen > prz->buffer_size)
 		size = prz->buffer_size - hlen;
-- 
2.20.1


-- 
Kees Cook

Re: [PATCH v4.4.z] pstore/ram: Write new dumps to start of recycled zones

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 2187 bytes --]

On Tue, 2020-01-07 at 10:26 -0800, Kees Cook wrote:
> From: Aleksandr Yashkin <a.yashkin@inango-systems.com>
> 
> [ Upstream commit 9e5f1c19800b808a37fb9815a26d382132c26c3d ]

This looks applicable to 3.16 as well, so I've queued it up.

Ben.

> The ram_core.c routines treat przs as circular buffers. When writing a
> new crash dump, the old buffer needs to be cleared so that the new dump
> doesn't end up in the wrong place (i.e. at the end).
> 
> The solution to this problem is to reset the circular buffer state before
> writing a new Oops dump.
> 
> Signed-off-by: Aleksandr Yashkin <a.yashkin@inango-systems.com>
> Signed-off-by: Nikolay Merinov <n.merinov@inango-systems.com>
> Signed-off-by: Ariel Gilman <a.gilman@inango-systems.com>
> Link: https://lore.kernel.org/r/20191223133816.28155-1-n.merinov@inango-systems.com
> Fixes: 896fc1f0c4c6 ("pstore/ram: Switch to persistent_ram routines")
> [kees: backport to v4.9]
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
>  fs/pstore/ram.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
> index 59d93acc29c7..fa0e89edb62d 100644
> --- a/fs/pstore/ram.c
> +++ b/fs/pstore/ram.c
> @@ -319,6 +319,17 @@ static int notrace ramoops_pstore_write_buf(enum pstore_type_id type,
>  
>  	prz = cxt->przs[cxt->dump_write_cnt];
>  
> +	/*
> +	 * Since this is a new crash dump, we need to reset the buffer in
> +	 * case it still has an old dump present. Without this, the new dump
> +	 * will get appended, which would seriously confuse anything trying
> +	 * to check dump file contents. Specifically, ramoops_read_kmsg_hdr()
> +	 * expects to find a dump header in the beginning of buffer data, so
> +	 * we must to reset the buffer values, in order to ensure that the
> +	 * header will be written to the beginning of the buffer.
> +	 */
> +	persistent_ram_zap(prz);
> +
>  	hlen = ramoops_write_kmsg_hdr(prz, compressed);
>  	if (size + hlen > prz->buffer_size)
>  		size = prz->buffer_size - hlen;
> -- 
> 2.20.1
> 
> 
-- 
Ben Hutchings
The program is absolutely right; therefore, the computer must be wrong.



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]