Re: [PATCH] ata: libata-sata: fix ata_scsi_lpm_supported() iteration
[email protected] Tue, 28 Jul 2026 11:53:36 +0000
| Newsgroups | gmane.linux.ide |
|---|---|
| Message-ID | <[email protected]> |
Thank you for your contribution! Sashiko AI review found 1 potential issue(= s) to consider: Pre-existing issues: - [High] ata_scsi_lpm_supported() skips checking devices on the slave link = because it uses the ATA_LITER_EDGE iterator. This allows userspace to enabl= e Link Power Management (LPM) on a port where the slave device does not sup= port it (has ATA_QUIRK_NOLPM). -- --- Patch [1]: [PATCH] ata: libata-sata: fix ata_scsi_lpm_supported() itera= tion --- commit 32bd728de2e1b6025504205f3ade16813231e703 Author: Niklas Cassel <[email protected]> ata: libata-sata: fix ata_scsi_lpm_supported() iteration This commit fixes the inner loop of ata_scsi_lpm_supported() to iterate over the specific link being evaluated rather than always using the host link. This prevents the LPM policy from being incorrectly applied or denied. Link: https://lore.kernel.org/linux-ide/[email protected]= rnel.org/ > diff --git a/drivers/ata/libata-sata.c b/drivers/ata/libata-sata.c > index b0706c30da052..ad40f516c6d43 100644 > --- a/drivers/ata/libata-sata.c > +++ b/drivers/ata/libata-sata.c > @@ -913,7 +913,7 @@ static bool ata_scsi_lpm_supported(struct ata_port *a= p) > return false; > =20 > ata_for_each_link(link, ap, EDGE) { > - ata_for_each_dev(dev, &ap->link, ENABLED) { > + ata_for_each_dev(dev, link, ENABLED) { [Severity: High] This is a pre-existing issue, but does using the ATA_LITER_EDGE iterator here in ata_scsi_lpm_supported() skip checking devices on the slave link? Since the EDGE iterator skips ap->slave_link, it appears this loop misses devices on the slave link that might have the ATA_QUIRK_NOLPM quirk.=20 If a user or power-management tool writes to the sysfs attribute link_power_management_policy, ata_scsi_lpm_supported() would incorrectly return true. Then ata_port_schedule_eh() triggers Error Handling, and ata_eh_link_set_lpm() might blindly apply the unsupported policy to the slave link.=20 Can this result in the slave drive becoming unresponsive, leading to command timeouts or dropped devices? > if (dev->quirks & ATA_QUIRK_NOLPM) > return false; > } --=20 Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260728113841.7272= [email protected]?part=3D1