linux-kernel - Re: [PATCH v2] pstore: pass allocated memory region back to caller

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:	Thu, 17 Nov 2011 08:53:37 -0500
From:	Don Zickus <dzickus@...hat.com>
To:	Tony Luck <tony.luck@...il.com>
Cc:	Kees Cook <keescook@...omium.org>, linux-kernel@...r.kernel.org,
	Matthew Garrett <mjg@...hat.com>,
	Chen Gong <gong.chen@...ux.intel.com>,
	Huang Ying <ying.huang@...el.com>,
	Mike Waychison <mikew@...gle.com>,
	Greg Kroah-Hartman <gregkh@...e.de>,
	Seiji Aguchi <seiji.aguchi@....com>
Subject: Re: [PATCH v2] pstore: pass allocated memory region back to caller

On Wed, Nov 16, 2011 at 02:45:24PM -0800, Tony Luck wrote:
> On Wed, Nov 16, 2011 at 2:20 PM, Don Zickus <dzickus@...hat.com> wrote:
> > This is an interesting approach.  But are we leaving psinfo data exposed
> > when you have a reader and writer at the same time?
> 
> I look at this as the first step in separating the read & write paths.
> 
> I started out with the (good) idea that the back end should allocate & own
> the buffer for the write path ... this means that the buffer is ready to use
> when an oops/panic happens - which is obviously a bad time to need to
> allocate memory :-)

Of course. :-)  But don't we have mechanisms which can use pre-allocated
memory?  Like the lock-less link list or the ring buffer?

Or maybe something safer, perhaps pass the backend buffer size to the
dumper routine when it registers?  That way kmsg_dump can allocate memory
at registration time which is sync'd in size with the backend.  This
allows kmsg_dump to fill the buffer and pass it directly down to the
backend (through pstore_dumper), with minimal locks, without breaking it
up again and re-copying into yet another buffer.

Thoughts?

> 
> Then I reused the same buffer for read - in hindsight this was not such a
> good idea - it led to all the discussions we've had about how to guarantee
> that the dmesg data gets saved on panic - even in the cases where we
> can't get the locks (so proposals have been made to bust the locks).
> 
> So Kees' patch is the functional equivalent of busting the spinlock.

> 
> Next step would be to look at the back end drivers to see whether
> they can handle a simultaneous read & write in a graceful way.
> 
I was just wondering if we should put a 'const' on the psinfo data being
passed to the read/write routine, otherwise a broken backend could modify
psinfo and corrupt any concurrent access, no?

> I've queued this for linux-next. Probably missed the snapshot today,
> but I expect it should show up in next-20111118

Cheers,
Don

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/