linux-kernel - Re: [RFC/RFT PATCH] PM: sleep: Ignore device driver suspend() callback return values

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: <CAPDyKFoJ45PZ_o6VdaCiyat+BC6XOZ5AMnxmsZVzk16cCxmDkw@mail.gmail.com>
Date: Thu, 5 Dec 2024 16:33:04 +0100
From: Ulf Hansson <ulf.hansson@...aro.org>
To: Alan Stern <stern@...land.harvard.edu>, "Rafael J. Wysocki" <rafael@...nel.org>, 
	Len Brown <lenb@...nel.org>
Cc: linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org, 
	Saravana Kannan <saravanak@...gle.com>
Subject: Re: [RFC/RFT PATCH] PM: sleep: Ignore device driver suspend()
 callback return values

On Thu, 5 Dec 2024 at 16:09, Alan Stern <stern@...land.harvard.edu> wrote:
>
> On Thu, Dec 05, 2024 at 12:55:08PM +0100, Rafael J. Wysocki wrote:
> > Expanded CC list.
> >
> > On Thu, Nov 14, 2024 at 4:23 AM Len Brown <lenb@...nel.org> wrote:
> > >
> > > From: Len Brown <len.brown@...el.com>
> > >
> > > Drivers commonly return non-zero values from their suspend
> > > callbacks due to transient errors, not realizing that doing so
> > > aborts system-wide suspend.
> > >
> > > Log, but do not abort system suspend on non-zero return values
> > > from driver's .suspend/.suspend_noirq/.suspend_late callbacks.
> > >
> > > Both before and after this patch, the correct method for a
> > > device driver to abort system-wide suspend is to invoke
> > > pm_system_wakeup() during the suspend flow.
> > >
> > > Legacy behaviour can be restored by adding this line to your .config:
> > > CONFIG_PM_SLEEP_LEGACY_CALLBACK_ABORT=y
> > >
> > > Signed-off-by: Len Brown <len.brown@...el.com>
> > > ---
>
> >
> > I'm wondering if there are any opinions on this.
> >
> > IMV, drivers returning errors from their suspend callbacks without a
> > sufficiently serious reason are kind of a problem.
>
> There is a least one driver whose suspend callback returns an error if
> the device is enabled for wakeup and a wakeup event occurs during the
> suspend procedure.  We don't want to ignore those races.
>
> Alan Stern

Right. I also think this looks a bit risky as the current behaviour
has really been there for a long time. Who knows what depends on this.

A way forward could be to implement the change as an opt-in thing,
rather than an opt-out. That would allow us to test it and see how it
plays to potentially change the default behaviour down the road.

Kind regards
Uffe