linux-kernel - Re: [PATCH] pstore/blk: Use the normal block device I/O path

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: <202106141436.294D1B2@keescook>
Date:   Mon, 14 Jun 2021 14:46:22 -0700
From:   Kees Cook <keescook@...omium.org>
To:     Al Viro <viro@...iv.linux.org.uk>
Cc:     Christoph Hellwig <hch@....de>,
        WeiXiong Liao <gmpy.liaowx@...il.com>, axboe@...nel.dk,
        Anton Vorontsov <anton@...msg.org>,
        Colin Cross <ccross@...roid.com>,
        Tony Luck <tony.luck@...el.com>, linux-kernel@...r.kernel.org,
        linux-block@...r.kernel.org, linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH] pstore/blk: Use the normal block device I/O path

On Mon, Jun 14, 2021 at 08:09:30PM +0000, Al Viro wrote:
> On Mon, Jun 14, 2021 at 01:04:21PM -0700, Kees Cook wrote:
>   
> >  static ssize_t psblk_generic_blk_write(const char *buf, size_t bytes,
> >  		loff_t pos)
> >  {
> 
> >  	/* Console/Ftrace backend may handle buffer until flush dirty zones */
> >  	if (in_interrupt() || irqs_disabled())
> >  		return -EBUSY;
> 
> > +	return kernel_write(psblk_file, buf, bytes, &pos);
> 
> In which locking environments could that be called?  The checks above
> look like that thing could be called from just about any context;
> could that happen when the caller is holding a page locked?

The contexts are determined by both each of the pstore "front ends":


PSTORE_FLAGS_DMESG:
static struct kmsg_dumper pstore_dumper = {
        .dump = pstore_dump,
...
kmsg_dump_register(&pstore_dumper);


PSTORE_FLAGS_CONSOLE:
static struct console pstore_console = {
        .write  = pstore_console_write,
...
register_console(&pstore_console);


PSTORE_FLAGS_FTRACE:
static struct ftrace_ops pstore_ftrace_ops __read_mostly = {
        .func   = pstore_ftrace_call,
...
register_ftrace_function(&pstore_ftrace_ops);


PSTORE_FLAGS_PMSG:
static const struct file_operations pmsg_fops = {
...
        .write          = write_pmsg,
...
pmsg_major = register_chrdev(0, PMSG_NAME, &pmsg_fops);


and each of the pstore "back ends". (ram, EFI vars, block, etc.)


> IOW, what are those checks really trying to do?

Traditionally, the most restrictive case is kmsg_dump, but that's the
whole point here of the "best effort" mode: if we can't safely make the
call and no panic handler has been registered, we must skip the call.

e.g. the RAM pstore backend has all its buffers preallocated, and it'll
just write directly into them. The handling here has gotten progressive
weirder, as more back ends landed -- i.e. EFI var writing added some
limits to the kind of locking pstore could do, etc.


It may turn out that the checks above aren't needed. I haven't tried it
without, but I suspect it's for the kmsg_dump case.

-Kees

-- 
Kees Cook