From: Niklas Cassel <cassel@kernel.org>
To: Damien Le Moal <dlemoal@kernel.org>
Cc: linux-ide@vger.kernel.org, linux-scsi@vger.kernel.org,
"Martin K . Petersen" <martin.petersen@oracle.com>,
Dan Carpenter <dan.carpenter@linaro.org>
Subject: Re: [PATCH] ata: libata-scsi: Fix ata_scsi_port_error_handler() error path
Date: Fri, 12 Apr 2024 14:08:08 +0200 [thread overview]
Message-ID: <ZhkkKPpuscTx4ZT0@ryzen> (raw)
In-Reply-To: <20240411234731.810968-1-dlemoal@kernel.org>
On Fri, Apr 12, 2024 at 08:47:31AM +0900, Damien Le Moal wrote:
> Commit 0c76106cb975 ("scsi: sd: Fix TCG OPAL unlock on system resume")
> incorrectly handles scsi_resume_device() errors, leading to a double
> call to spin_unlock_irqrestore() to unlock a device port. Fix this by
> redefining the goto labels used in case of error and only unlock the
> port scsi_scan_mutex when scsi_resume_device() fails.
>
> Bug found with the Smatch static checker warning:
>
> drivers/ata/libata-scsi.c:4774 ata_scsi_dev_rescan()
> error: double unlocked 'ap->lock' (orig line 4757)
>
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Fixes: 0c76106cb975 ("scsi: sd: Fix TCG OPAL unlock on system resume")
> Cc: stable@vger.kernel.org
> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
> ---
> drivers/ata/libata-scsi.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index 2f4c58837641..e954976891a9 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -4745,7 +4745,7 @@ void ata_scsi_dev_rescan(struct work_struct *work)
> * bail out.
> */
> if (ap->pflags & ATA_PFLAG_SUSPENDED)
> - goto unlock;
> + goto unlock_ap;
>
> if (!sdev)
> continue;
> @@ -4758,7 +4758,7 @@ void ata_scsi_dev_rescan(struct work_struct *work)
> if (do_resume) {
> ret = scsi_resume_device(sdev);
> if (ret == -EWOULDBLOCK)
> - goto unlock;
> + goto unlock_scan;
> dev->flags &= ~ATA_DFLAG_RESUMING;
> }
> ret = scsi_rescan_device(sdev);
> @@ -4766,12 +4766,13 @@ void ata_scsi_dev_rescan(struct work_struct *work)
> spin_lock_irqsave(ap->lock, flags);
>
> if (ret)
> - goto unlock;
> + goto unlock_ap;
> }
> }
>
> -unlock:
> +unlock_ap:
> spin_unlock_irqrestore(ap->lock, flags);
> +unlock_scan:
> mutex_unlock(&ap->scsi_scan_mutex);
>
> /* Reschedule with a delay if scsi_rescan_device() returned an error */
> --
> 2.44.0
>
Subject: [PATCH] ata: libata-scsi: Fix ata_scsi_port_error_handler() error path
ata_scsi_port_error_handler() ? How did you come up with that? :)
Wrong copy paste?
s/ata_scsi_port_error_handler()/ata_scsi_dev_rescan()/
With that:
Reviewed-by: Niklas Cassel <cassel@kernel.org>
prev parent reply other threads:[~2024-04-12 12:08 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-11 23:47 [PATCH] ata: libata-scsi: Fix ata_scsi_port_error_handler() error path Damien Le Moal
2024-04-12 12:08 ` Niklas Cassel [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZhkkKPpuscTx4ZT0@ryzen \
--to=cassel@kernel.org \
--cc=dan.carpenter@linaro.org \
--cc=dlemoal@kernel.org \
--cc=linux-ide@vger.kernel.org \
--cc=linux-scsi@vger.kernel.org \
--cc=martin.petersen@oracle.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.