linux-kernel - RE: [PATCH 06/13] ACPI/IPMI: Add reference counting for ACPI operation region handlers

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: <1AE640813FDE7649BE1B193DEA596E8802435E11@SHSMSX101.ccr.corp.intel.com>
Date:	Fri, 26 Jul 2013 08:15:56 +0000
From:	"Zheng, Lv" <lv.zheng@...el.com>
To:	"Zheng, Lv" <lv.zheng@...el.com>, "Rafael J. Wysocki" <rjw@...k.pl>
CC:	"Wysocki, Rafael J" <rafael.j.wysocki@...el.com>,
	"Brown, Len" <len.brown@...el.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>
Subject: RE: [PATCH 06/13] ACPI/IPMI: Add reference counting for ACPI
 operation region handlers

> From: linux-acpi-owner@...r.kernel.org
> [mailto:linux-acpi-owner@...r.kernel.org] On Behalf Of Zheng, Lv
> Sent: Friday, July 26, 2013 9:54 AM
> To: Rafael J. Wysocki
> Cc: Wysocki, Rafael J; Brown, Len; linux-kernel@...r.kernel.org;
> linux-acpi@...r.kernel.org
> Subject: RE: [PATCH 06/13] ACPI/IPMI: Add reference counting for ACPI
> operation region handlers
> 
> > From: Rafael J. Wysocki [mailto:rjw@...k.pl]
> > Sent: Friday, July 26, 2013 5:29 AM
> >
> > On Tuesday, July 23, 2013 04:09:43 PM Lv Zheng wrote:
> > > This patch adds reference couting for ACPI operation region handlers
> > > to fix races caused by the ACPICA address space callback invocations.
> > >
> > > ACPICA address space callback invocation is not suitable for Linux
> > > CONFIG_MODULE=y execution environment.
> >
> > Actually, can you please explain to me what *exactly* the problem is?
> 
> OK.  I'll add race explanations in the next revision.
> 
> The problem is there is no "lock" held inside ACPICA for invoking operation
> region handlers.
> Thus races happens between the acpi_remove/install_address_space_handler
> and the handler/setup callbacks.

This seems not a good explanation of the intent of this patch.
I think the intent is here in the patch description:

1. It acts as a barrier for operation region callbacks - no callback will
   happen after acpi_unregister_region().
2. acpi_unregister_region() is safe to be called in moudle->exit()
   functions.

Hmm, maybe I need to re-order the patch description for this patch.

Thanks for commenting.

Best regards
-Lv

> 
> This is correct per ACPI specification.
> As if there is interpreter locks held for invoking operation region handlers, the
> timeout implemented inside the operation region handlers will make all locking
> facilities (Acquire or Sleep,...) timed out.
> Please refer to ACPI specification "5.5.2 Control Method Execution":
> Interpretation of a Control Method is not preemptive, but it can block. When a
> control method does block, OSPM can initiate or continue the execution of a
> different control method. A control method can only assume that access to
> global objects is exclusive for any period the control method does not block.
> 
> So it is pretty much likely that ACPI IO transfers are locked inside the operation
> region callback implementations.
> Using locking facility to protect the callback invocation will risk dead locks.
> 
> Thanks
> -Lv
> 
> > Rafael