linux-kernel - RE: [PATCH V2] Change ACPI IPMI support to "default y"

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:	Mon, 24 Feb 2014 00:48:42 +0000
From:	"Zheng, Lv" <lv.zheng@...el.com>
To:	Matthew Garrett <matthew.garrett@...ula.com>
CC:	"rja@....com" <rja@....com>, "lenb@...nel.org" <lenb@...nel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"minyard@....org" <minyard@....org>,
	"rjw@...ysocki.net" <rjw@...ysocki.net>,
	"linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>
Subject: RE: [PATCH V2] Change ACPI IPMI support to "default y"

Hi,

> From: linux-acpi-owner@...r.kernel.org [mailto:linux-acpi-owner@...r.kernel.org] On Behalf Of Matthew Garrett
> Sent: Saturday, February 22, 2014 12:13 AM
> 
> On Fri, 2014-02-21 at 02:17 +0000, Zheng, Lv wrote:
> 
> > In fact there is a workaround solution I've posted here:
> > https://patchwork.kernel.org/patch/2831851/
> > The updated version of this patch can be found at:
> > https://bugzilla.kernel.org/attachment.cgi?id=112611
> > It is the acpi-ipmi13.patch file.
> 
> I don't think that solves the underlying problem? We're still left with
> no guarantee of ordering between ACPI IPMI operation region support
> being available and the loading of a driver that may rely on it.

Yes. As it is a bit far beyond the bug report, I didn't work on it.
But actually, I have solution to achieve this.

> > I think there should be relationship between ACPI region and Linux kernel modules.
> > In fact on the well-known operating system, _REG is always invoked at the end of the IRP_PNP_START_DEVICE completions.
> > But we may still be able to return -EPROBE_DEFER in the power meter driver when it detects failures caused by the readiness state
> of the region handlers.
> 
> We don't have a guarantee that it'll happen at probe time. The
> capabilities list may be static, and so we'll get our first failure on
> an attempted userspace access instead.

This is the issue related to the IPMI operation region, and it can be solved by a "request_module()" call.
The -EPROBE_DEFER I mentioned is about the quality of  acpi power meter driver itself.

> It may be that Corey's suggestion satisfies the concerns Russ raised.
> Building IPMI in but only probing by default on systems that declare a
> BMC in ACPI or DMI should avoid boot-time delays while still ensuring
> that the functionality is available on systems where there's a real
> chance that the firmware expects the OS to provide support.

If you take a look at this patch (https://patchwork.kernel.org/patch/2831851/), you'll see it is possible to add request_module() in acpi_region_default_handler().
The callback is invoked when the first IPMI region access happens and it is in the process context without any locking.
I'd rather to embed acpi_ipmi into ipmi_si module with this fix:
https://bugzilla.kernel.org/attachment.cgi?id=112611 (acpi-ipmi14.patch).
After that, a request_module() invocation of ipmi_si in acpi_region_default_handler() can solve what you are worrying about.

What's your opinion?

Thanks and best regards
-Lv

> --
> Matthew Garrett <matthew.garrett@...ula.com>