linux-kernel - Re: [PATCH] scsi: storvsc: Fix a panic in the hibernation procedure

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: <f23cb660-13e4-8466-4c78-163fcc857caa@acm.org>
Date:   Thu, 23 Apr 2020 16:25:11 -0700
From:   Bart Van Assche <bvanassche@....org>
To:     Dexuan Cui <decui@...rosoft.com>,
        "jejb@...ux.ibm.com" <jejb@...ux.ibm.com>,
        "martin.petersen@...cle.com" <martin.petersen@...cle.com>,
        "linux-scsi@...r.kernel.org" <linux-scsi@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "hch@....de" <hch@....de>, "hare@...e.de" <hare@...e.de>,
        Michael Kelley <mikelley@...rosoft.com>,
        Long Li <longli@...rosoft.com>,
        "ming.lei@...hat.com" <ming.lei@...hat.com>,
        Balsundar P <Balsundar.P@...rochip.com>
Cc:     "linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
        "wei.liu@...nel.org" <wei.liu@...nel.org>,
        Stephen Hemminger <sthemmin@...rosoft.com>,
        Haiyang Zhang <haiyangz@...rosoft.com>,
        KY Srinivasan <kys@...rosoft.com>
Subject: Re: [PATCH] scsi: storvsc: Fix a panic in the hibernation procedure

On 2020-04-23 11:29, Dexuan Cui wrote:
> So it looks the below patch also works for me:
> 
> --- a/kernel/power/hibernate.c
> +++ b/kernel/power/hibernate.c
> @@ -898,6 +898,11 @@ static int software_resume(void)
>         error = freeze_processes();
>         if (error)
>                 goto Close_Finish;
> +
> +       error = freeze_kernel_threads();
> +       if (error)
> +               goto Close_Finish;
> +
>         error = load_image_and_restore();
>         thaw_processes();
>   Finish:
> 
> Just to be sure, I'll do more tests, but I believe the panic can be fixed
> by this according to my tests I have done so far.

If a freeze_kernel_threads() call is added in software_resume(), should
a thaw_kernel_threads() call be added too?

Anyway, please Cc me if a patch for software_resume() is submitted.

> I'm still not sure what the comment before scsi_device_quiesce() means:
>  *  ... Since special requests may also be requeued requests,
>  *      a successful return doesn't guarantee the device will be
>  *      totally quiescent.
> 
> I don't know if there can be some other I/O submitted after
> scsi_device_quiesce() returns in the case of hibernation, and I don't
> know if aac_suspend() -> scsi_host_block() should be fixed/removed,
> but as far as the panic is concerned, I'm very glad I have found a better
> fix with your help. 

The function blk_set_pm_only() increments the q->pm_only counter while
the blk_clear_pm_only() function decrements the q->pm_only counter.
If q->pm_only > 0, blk_queue_enter() only succeeds if the flag
BLK_MQ_REQ_PREEMPT is set in the second argument passed to that
function. blk_get_request() calls blk_queue_enter(). The result is that
while q->pm_only > 0 blk_get_request() only submits a request without
waiting if the BLK_MQ_REQ_PREEMPT flag is set in its second argument.
scsi_execute() sets the BLK_MQ_REQ_PREEMPT flag. In other words,
scsi_device_quiesce() blocks requests submitted by filesystems but still
allows SCSI commands submitted by the SCSI core to be executed.
"special" refers to requests with the BLK_MQ_REQ_PREEMPT flag set.

Bart.