Re: [PATCH] pstore/ram: fix for adding dumps to non-empty zone

[Kees Cook <keescook@chromium.org>]

On Mon, Dec 23, 2019 at 06:38:16PM +0500, Nikolai Merinov wrote:
> From: Aleksandr Yashkin <a.yashkin@inango-systems.com>
> 
> The circle buffer in ramoops zones has a problem for adding a new
> oops dump to already an existing one.
> 
> The solution to this problem is to reset the circle buffer state before
> writing a new oops dump.

Ah, I see it now. When the crashes wrap around, the header is written at
the end of the (possibly incompletely filled) buffer, instead of at the
start, since it wasn't explicitly zapped.

Yes, this is important; thank you for tracking this down! This bug has
existed for a very long time. I'll try to find the right Fixes tag for
it...

-Kees

> 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>
> 
> diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
> index 8caff834f002..33fceadbf515 100644
> --- a/fs/pstore/ram.c
> +++ b/fs/pstore/ram.c
> @@ -407,6 +407,13 @@ static int notrace ramoops_pstore_write(struct pstore_record *record)
>  
>  	prz = cxt->dprzs[cxt->dump_write_cnt];
>  
> +	/* Clean the buffer from old info.
> +	 * `ramoops_read_kmsg_hdr' expects to find a 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);
> +
>  	/* Build header and append record contents. */
>  	hlen = ramoops_write_kmsg_hdr(prz, record);
>  	if (!hlen)
> -- 
> 2.17.1
> 

-- 
Kees Cook