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: <aJ2vIRxStgzJACFQ@ryzen>
Date: Thu, 14 Aug 2025 11:40:49 +0200
From: Niklas Cassel <cassel@...nel.org>
To: Igor Pylypiv <ipylypiv@...gle.com>
Cc: Damien Le Moal <dlemoal@...nel.org>, linux-ide@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] ata: libata-scsi: Fix CDL control

On Wed, Aug 13, 2025 at 07:22:56PM -0700, Igor Pylypiv wrote:
> Delete extra checks for the ATA_DFLAG_CDL_ENABLED flag that prevent
> SET FEATURES command from being issued to a drive when NCQ commands
> are active.
> 
> ata_mselect_control_ata_feature() sets / clears the ATA_DFLAG_CDL_ENABLED
> flag during the translation of MODE SELECT to SET FEATURES. If SET FEATURES
> gets deferred due to outstanding NCQ commands, the original MODE SELECT
> command will be re-queued. When the re-queued MODE SELECT goes through
> the ata_mselect_control_ata_feature() translation again, SET FEATURES
> will not be issued because ATA_DFLAG_CDL_ENABLED has been already set or
> cleared by the initial translation of MODE SELECT.
> 
> The ATA_DFLAG_CDL_ENABLED checks in ata_mselect_control_ata_feature()
> are safe to remove because scsi_cdl_enable() implements a similar logic
> that avoids enabling CDL if it has been enabled already.
> 
> Fixes: 17e897a45675 ("ata: libata-scsi: Improve CDL control")
> Signed-off-by: Igor Pylypiv <ipylypiv@...gle.com>

Reviewed-by: Niklas Cassel <cassel@...nel.org>

However, like Damien said, we really need to improve the API so that the
SAT can know if the command was success or failure.

That way we could set the flag only after the command was successful.


Kind regards,
Niklas

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ