linux-kernel - Re: [PATCH v2 0/2] extend kexec_file

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, 18 Aug 2016 11:21:13 +0100
From:	Mark Rutland <mark.rutland@....com>
To:	Thiago Jung Bauermann <bauerman@...ux.vnet.ibm.com>
Cc:	kexec@...ts.infradead.org, linuxppc-dev@...ts.ozlabs.org,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
	AKASHI Takahiro <takahiro.akashi@...aro.org>,
	Eric Biederman <ebiederm@...ssion.com>,
	Dave Young <dyoung@...hat.com>,
	Vivek Goyal <vgoyal@...hat.com>, Baoquan He <bhe@...hat.com>,
	David Laight <David.Laight@...LAB.COM>,
	Michael Ellerman <mpe@...erman.id.au>,
	Benjamin Herrenschmidt <benh@...nel.crashing.org>,
	Stewart Smith <stewart@...ux.vnet.ibm.com>,
	Arnd Bergmann <arnd@...db.de>,
	Russell King - ARM Linux <linux@...linux.org.uk>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH v2 0/2] extend kexec_file_load system call

On Thu, Aug 11, 2016 at 08:03:56PM -0300, Thiago Jung Bauermann wrote:
> This patch series is from AKASHI Takahiro. I will use it in my next
> version of the kexec_file_load implementation for powerpc, so I am
> rebasing it on top of v4.8-rc1.

[...]

> Original cover letter:
> 
> Device tree blob must be passed to a second kernel on DTB-capable
> archs, like powerpc and arm64, but the current kernel interface
> lacks this support.
> 
> This patch extends kexec_file_load system call by adding an extra
> argument to this syscall so that an arbitrary number of file descriptors
> can be handed out from user space to the kernel.
> 
> See the background [1].
> 
> Please note that the new interface looks quite similar to the current
> system call, but that it won't always mean that it provides the "binary
> compatibility."
> 
> [1] http://lists.infradead.org/pipermail/kexec/2016-June/016276.html

As with the original posting, I have a number of concerns, and I'm
really not keen on this.

* For typical usecases, I do not believe that this is necessary (at
  least for arm64), and generally do not believe that it should be
  necessary for a user to manipulate the DTB (much like the user need
  not manipulate ACPI tables or other FW data structures).

  Other than (potentially) the case of Linux as a flashed-in bootloader,
  I don't see a compelling case for modifying the DTB that could not be
  accomplished in-kernel. For that case, if truly necessary, I think
  that we can get away with something simpler.

* This series adds architecture-specific hooks, but doesn't define what
  the architecture code is expected to do. For example, what is the
  format of the partial DTB? Is it formatted as an overlay, or a regular
  DTB that is expected to be merged somehow?

  I'm afraid that the scope is unbound, and we'll see requests to
  whitelist/blacklist arbitrary nodes or properties in arch code. This
  goes against the original simple design of kexec_file_load. It also
  implies that we're going to have varied architecture-specific
  semantics, and that arch code might not consistently check all that it
  should.

* Further, I believe that this offers a lot of scope for unintentionally
  allowing certain modifications to the DTB that we do not want, and
  avoiding that in general is very tricky. e.g. if we allow the
  insertion or modification of nodes, how do we prevent phandle target
  hijacking?

I really don't think that this is a good idea. Please consider this a
NAK from my end.

Thanks,
Mark.