linux-kernel - Re: [PATCH v3] dell-rbtn: Ignore ACPI notifications if device is suspended

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, 24 May 2016 12:57:26 -0700
From:	Darren Hart <dvhart@...radead.org>
To:	Pali Rohár <pali.rohar@...il.com>
Cc:	Andrei Borzenkov <arvidjaar@...il.com>,
	Gabriele Mazzotta <gabriele.mzt@...il.com>,
	"Rafael J. Wysocki" <rjw@...ysocki.net>,
	"D. Jared Dominguez" <Jared_Dominguez@...l.com>,
	"platform-driver-x86@...r.kernel.org" 
	<platform-driver-x86@...r.kernel.org>,
	Alex Hung <alex.hung@...onical.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3] dell-rbtn: Ignore ACPI notifications if device is
 suspended

On Tue, May 24, 2016 at 09:09:38AM +0200, Pali Rohár wrote:
> On Tuesday 24 May 2016 06:48:41 Andrei Borzenkov wrote:
> > 24.05.2016 02:03, Gabriele Mazzotta пишет:
> > > On 24/05/2016 00:22, Pali Rohár wrote:

...

> > > +#ifdef CONFIG_PM_SLEEP
> > > +static void ACPI_SYSTEM_XFACE rbtn_acpi_clear_flag(void *context)
> 
> I would rename this function to rbtn_clear_suspended_flag.
> 

...

> > > +	/*
> > > +	 * Upon resume, some BIOSes autonomously send an ACPI notification

You can drop "autonomously", it reads a bit awkwardly, and doesn't add any
information.

> > > +	 * that triggers an unwanted input event. In order to ignore it,
> > > +	 * we use a flag that we set at suspend and clear once we have
> > > +	 * received the extra notification. Since ACPI notifications are
> > > +	 * delivered asynchronously to drivers, we clear the flag from the
> > > +	 * workqueue used to deliver the notifications. This should be enough
> > > +	 * to guarantee that the flag is cleared only after we received the
> > > +	 * extra notification, if any.
> > > +	 */
> > 
> > "guarantee" is rather strong word here. We really do not know anything
> > how and when these notifications are generated by firmware, so can only
> > hope. But otherwise this explains what this patch intends to do (so that
> > even me finally understood it :)
> 
> Yes, thats better.
> 
> > > +	status = acpi_os_execute(OSL_NOTIFY_HANDLER,
> > > +			 rbtn_acpi_clear_flag, rbtn_data);
> > > +	if (ACPI_FAILURE(status))
> > > +		rbtn_data->suspended = false;
> 
> And here rbtn_clear_suspended_flag(rbtn_data) call instead direct
> assignment.
> 

I'm dropping this from the queue, and awaiting an updated version with the
requested changes (these from Pali, and the issue raised about "guarantee" being
too strong).

Thanks,

-- 
Darren Hart
Intel Open Source Technology Center