linux-kernel - Re: efi_pstore: question about how to remove create_sysfs

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:	Tue, 14 Aug 2012 11:04:00 -0700
From:	Mike Waychison <mikew@...gle.com>
To:	Seiji Aguchi <seiji.aguchi@....com>
Cc:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"Luck, Tony (tony.luck@...el.com)" <tony.luck@...el.com>,
	"Matthew Garrett (mjg@...hat.com)" <mjg@...hat.com>,
	"dzickus@...hat.com" <dzickus@...hat.com>,
	"dle-develop@...ts.sourceforge.net" 
	<dle-develop@...ts.sourceforge.net>,
	Satoru Moriya <satoru.moriya@....com>
Subject: Re: efi_pstore: question about how to remove create_sysfs_entry()
 from a write callback.

On Tue, Aug 14, 2012 at 9:05 AM, Seiji Aguchi <seiji.aguchi@....com> wrote:
> Hi,
>
> I'm sending an email to discuss how to remove create_sysfs_entry() from a write callback.
>
> [Problem]
>
> Current efi_pstore creates sysfs entries ,which enable users to access to NVRAM, in a write callback.
> If a kernel panic happens in interrupt contexts, pstore may fail because it could sleep due to dynamic
> memory allocations during creating sysfs entries.
>
> To resolve the problem above, my goal here is removing create_sysfs_entry() from a write callback.
>
> [Ideas]
>
>  (1) Introduce a workqueue updating sysfs entries
>
>      To remove create_sysfs_entry() from a write callback,
>      It seems to be possible if efi_pstore updates its sysfs files
>      by scanning existing entries in NVRAM with a GetNextVariable()
>      in a workqueue.
>
>
>      I created a prototype patch based on an idea above but can't avoid a race
>      between SetVariable() in a write callback and GetNextVariable() in a workqueue.
>      It is not guaranteed by EFI specification.
>
>      EFI 2.3.1 specification, page 217.
>      <snip>
>      Calls to SetVariable() between calls to
>      GetNextVariableName() may produce unpredictable results.
>      <snip>

Can we not serialize this with &efivars->lock if it is updated to
disable interrupts?  A loop in the workqueue that locks, iterates
through ->get_next_variable, and compares against searches in
efivars->list should work, no?

>
>
>  (2) Don't support sysfs entries in efi_pstore.
>
>      Another idea is _not_ updating sysfs entries at all in efi_pstore.
>      This can avoid a race SetVariable() and GetNextVariable().
>
>      write callback
>        - simply write a new entry with SetVariable().
>          - This fits a discussion about holding multiple logs in a thread below.
>            http://marc.info/?l=linux-kernel&m=134316268011854&w=2
>
>      erase callback
>        - simply erase an existing entry with SetVariable().
>
>      read callback
>        - Scaning existing entries with GetNextVariable().
>          We can avoid a race between GetNextVariable() in a read callback
>          and SetVariable() in a write/erase callback by protecting them with efi_lock.
>
>  IMO, idea (2) is reasonable because we already have an interface, /dev/pstore, which users can access
>  to NVRAM and we don't need to support multiple user interfaces.
>
> Any comments are welcome.
>
> Seiji
--
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/