linux-kernel - Re: [PATCH] pm: sleep: do not set is_prepared when no_pm

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: <CAJZ5v0hMnnDjKJLMgcT_p1nnejyyAyaqaA_AF5t+_=PsSMfceQ@mail.gmail.com>
Date: Tue, 3 Sep 2024 14:31:04 +0200
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: David Wang <00107082@....com>
Cc: rafael@...nel.org, pavel@....cz, gregkh@...uxfoundation.org, 
	linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] pm: sleep: do not set is_prepared when no_pm_callbacks is set

On Mon, Sep 2, 2024 at 2:59 PM David Wang <00107082@....com> wrote:
>
> When resume, a parent device with no pm callbacks
> would have "is_prepared" and "direct_complete" bit
> set, and skip the "fib" chance to unset "is_prepared"
> in device_resume because of the direct_complete bit.

Sure, but is_prepared will be cleared in device_complete() AFAICS.

> This will trigger a kernel warning when resume its child
> For example, when suspend system with an USB webcam
> opened, following warning would show up during resume:
>
>  >usb 3-1.1: reset high-speed USB device number 4 using xhci_hcd
>  >..
>  >ep_81: PM: parent 3-1.1:1.1 should not be sleeping

This is printed in device_pm_add(), so apparently something new has
appeared under the parent while it's between "resume" and "prepare".

The parent is actually still regarded as "suspended" because any
resume callbacks have not been called for it, but new children can be
added under it at this point because doing so does not break the
dpm_list ordering and all of its ancestors have been already resumed.

> The device parenting relationships are:
> [usb 3-1.1] << [uvcvideo 3-1.1:1.1] << [ep_81].
> When resume, since the virtual [uvcvideo 3-1.1:1.1] device
> has no pm callbacks, it would not clear "is_prepared"
> once set.  Then, when resume [ep_81], pm module would
> yield a warn seeing [ep_81]'s parent [uvcvideo 3-1.1:1.1]
> having "is_prepared".
>
> Do not set "is_prepared" for virtual devices having
> no pm callbacks can clear those kernel warnings.
>
> Signed-off-by: David Wang <00107082@....com>
> ---
>  drivers/base/power/main.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
> index 934e5bb61f13..e2149ccf2c3e 100644
> --- a/drivers/base/power/main.c
> +++ b/drivers/base/power/main.c
> @@ -1880,7 +1880,8 @@ int dpm_prepare(pm_message_t state)
>                 mutex_lock(&dpm_list_mtx);
>
>                 if (!error) {
> -                       dev->power.is_prepared = true;
> +                       if (!dev->power.no_pm_callbacks)
> +                               dev->power.is_prepared = true;

This is not the way to address the issue IMV.

power.is_prepared set means that the device is in dpm_prepared_list
and I wouldn't depart from that even for devices without PM callbacks.

>                         if (!list_empty(&dev->power.entry))
>                                 list_move_tail(&dev->power.entry, &dpm_prepared_list);
>                 } else if (error == -EAGAIN) {
> --

It would be better to add a power.no_pm_callbacks check for the parent
to device_pm_add(), but this would still suppress the warning is some
cases in which it should be printed (for example, the new device's
parent is a "virtual" device without PM callbacks, but its grandparent
is a regular device that has PM callbacks and is suspended).

Something like the attached patch (untested) might work, though.

View attachment "pm-sleep-pm-add.patch" of type "text/x-patch" (1232 bytes)