linux-kernel - Re: [PATCH v2 2/2] firmware: exynos-acpm: allow use during system shutdown

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <974ddabee5a2a43b9d32f382ec4b13afab066f1a.camel@linaro.org>
Date: Tue, 25 Mar 2025 08:01:54 +0000
From: André Draszik <andre.draszik@...aro.org>
To: Krzysztof Kozlowski <krzk@...nel.org>, Tudor Ambarus
	 <tudor.ambarus@...aro.org>, Alim Akhtar <alim.akhtar@...sung.com>
Cc: Peter Griffin <peter.griffin@...aro.org>, Will McVicker
	 <willmcvicker@...gle.com>, kernel-team@...roid.com, 
	linux-kernel@...r.kernel.org, linux-samsung-soc@...r.kernel.org, 
	linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v2 2/2] firmware: exynos-acpm: allow use during system
 shutdown

Hi Krzysztof,

On Tue, 2025-03-25 at 08:57 +0100, Krzysztof Kozlowski wrote:
> On 24/03/2025 16:34, André Draszik wrote:
> > +static bool acpm_may_sleep(void)
> > +{
> > +	return system_state <= SYSTEM_RUNNING ||
> > +		(IS_ENABLED(CONFIG_PREEMPT_COUNT) ? preemptible() : !irqs_disabled());
> > +}
> > +
> >  /**
> >   * acpm_dequeue_by_polling() - RX dequeue by polling.
> >   * @achan:	ACPM channel info.
> > @@ -300,7 +314,10 @@ static int acpm_dequeue_by_polling(struct acpm_chan *achan,
> >  			return 0;
> >  
> >  		/* Determined experimentally. */
> > -		usleep_range(20, 30);
> > +		if (!acpm_may_sleep())
> > +			udelay(10);
> > +		else
> 
> ... and what do you do if IRQs get disabled exactly in this moment? This
> is just racy. You cannot check for a condition and assume it will be
> valid for whatever time you want it to be valid.
> 
> What happens if system_state is changed to shutdown in this particular
> moment? How did you prevent this from happening?

Yes, and that's also what the I2C subsystem is doing, AFAICS, see
i2c_in_atomic_xfer_mode() and its use. This is to make a very
specific corner case work, similar to I2C which has to deal with
the same issue during shutdown.

Would you have a better suggestion?


Cheers,
Andre'