linux-kernel - Re: [PATCH 2/2] printk: ringbuffer: avoid memcpy() on state

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]

Message-ID: <20200914124404.GD11154@alley>
Date:   Mon, 14 Sep 2020 14:44:05 +0200
From:   Petr Mladek <pmladek@...e.com>
To:     John Ogness <john.ogness@...utronix.de>
Cc:     Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>,
        Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
        Steven Rostedt <rostedt@...dmis.org>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] printk: ringbuffer: avoid memcpy() on state_var

On Mon 2020-09-14 11:54:03, John Ogness wrote:
> @state_var is copied as part of the descriptor copying via
> memcpy(). This is not allowed because @state_var is an atomic type,
> which in some implementations may contain a spinlock.

Great catch!

> Avoid using memcpy() with @state_var by explicitly copying the other
> fields of the descriptor. @state_var is set using atomic set
> operator before returning.

Just thinking loudly.

Strictly speaking, memcpy() might be used when the atomic variable
is later corrected by an atomic operation. And it is done by the first
patch.

I think that it is a matter of taste what solution is more error
prone. Either this function must be updated when a new field
is added into struct prb_desc or the atomic_long_set() must not
get removed.

Hmm, missing memcpy() should be rather easy to debug. While missing
atomic_long_set() is really subtle problem problem. So, the new
code is better from my POV.

> Fixes: b6cf8b3f3312 ("printk: add lockless ringbuffer")
> Signed-off-by: John Ogness <john.ogness@...utronix.de>

Reviewed-by: Petr Mladek <pmladek@...e.com>

Best Regards,
Petr