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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 30 Dec 2019 12:37:10 -0800
From:   Kees Cook <keescook@...omium.org>
To:     Nikolai Merinov <n.merinov@...ngo-systems.com>
Cc:     Anton Vorontsov <anton@...msg.org>,
        Colin Cross <ccross@...roid.com>,
        Tony Luck <tony.luck@...el.com>, linux-kernel@...r.kernel.org,
        Aleksandr Yashkin <a.yashkin@...ngo-systems.com>,
        Ariel Gilman <a.gilman@...ngo-systems.com>
Subject: Re: [PATCH] pstore/ram: fix for adding dumps to non-empty zone

On Mon, Dec 23, 2019 at 06:38:16PM +0500, Nikolai Merinov wrote:
> From: Aleksandr Yashkin <a.yashkin@...ngo-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@...ngo-systems.com>
> Signed-off-by: Nikolay Merinov <n.merinov@...ngo-systems.com>
> Signed-off-by: Ariel Gilman <a.gilman@...ngo-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