On Mon, Aug 12, 2024 at 05:15:18PM +0200, Niklas Cassel wrote:
> Sense data can be in either fixed format or descriptor format.
>
> SAT-6 revision 1, 10.4.6 Control mode page, says that if the D_SENSE bit
> is set to zero (i.e., fixed format sense data), then the SATL should
> return fixed format sense data for ATA PASS-THROUGH commands.
>
> A lot of user space programs incorrectly assume that the sense data is in
> descriptor format, without checking the RESPONSE CODE field of the
> returned sense data (to see which format the sense data is in).
>
> The libata SATL has always kept D_SENSE set to zero by default.
> (It is however possible to change the value using a MODE SELECT command.)
>
> For failed ATA PASS-THROUGH commands, we correctly generated sense data
> according to the D_SENSE bit. However, because of a bug, sense data for
> successful ATA PASS-THROUGH commands was always generated in the
> descriptor format.
>
> This was fixed to consistently respect D_SENSE for both failed and
> successful ATA PASS-THROUGH commands in commit 28ab9769117c ("ata:
> libata-scsi: Honor the D_SENSE bit for CK_COND=1 and no error").
>
> After commit 28ab9769117c ("ata: libata-scsi: Honor the D_SENSE bit for
> CK_COND=1 and no error"), we started receiving bug reports that we broke
> these user space programs (these user space programs must never have
> encountered a failing command, as the sense data for failing commands has
> always correctly respected D_SENSE, which by default meant fixed format).
>
> Since a lot of user space programs seem to assume that the sense data is
> in descriptor format (without checking the type), let's simply change the
> default to have D_SENSE set to one by default.
>
> That way:
> -Broken user space programs will see no regression.
> -Both failed and successful ATA PASS-THROUGH commands will respect D_SENSE,
> as per SAT-6 revision 1.
> -Apparently it seems way more common for user space applications to assume
> that the sense data is in descriptor format, rather than fixed format.
> (A user space program should of course support both, and check the
> RESPONSE CODE field to see which format the returned sense data is in.)
>
> Cc: stable@xxxxxxxxxxxxxxx # 4.19+
> Reported-by: Stephan Eisvogel <eisvogel@xxxxxxxxxx>
> Reported-by: Christian Heusel <christian@xxxxxxxxx>
> Closes: https://lore.kernel.org/linux-ide/0bf3f2f0-0fc6-4ba5-a420-c0874ef82d64@xxxxxxxxx/
> Fixes: 28ab9769117c ("ata: libata-scsi: Honor the D_SENSE bit for CK_COND=1 and no error")
> Signed-off-by: Niklas Cassel <cassel@xxxxxxxxxx>
> ---
> drivers/ata/libata-core.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index c7752dc80028..590bebe1354d 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
> @@ -5368,6 +5368,13 @@ void ata_dev_init(struct ata_device *dev)
> */
> spin_lock_irqsave(ap->lock, flags);
> dev->flags &= ~ATA_DFLAG_INIT_MASK;
> +
> + /*
> + * A lot of user space programs incorrectly assume that the sense data
> + * is in descriptor format, without checking the RESPONSE CODE field of
> + * the returned sense data (to see which format the sense data is in).
> + */
> + dev->flags |= ATA_DFLAG_D_SENSE;
> dev->horkage = 0;
> spin_unlock_irqrestore(ap->lock, flags);
>
> --
> 2.46.0
>
This patch will change so that the sense data will be generated in descriptor
format (by default) for passthrough (SG_IO) commands, not just SG_IO ATA
PASS-THROUGH commands.
Non-passthrough (SG_IO) commands are not relavant, as they will go via
scsi_finish_command(), which calls scsi_normalize_sense() before interpreting
the sense data, and for non-passthrough commands, the sense data is not
propagated to the user. (The SK/ASC/ASCQ is only printed to the log, and this
print will be the same as before.)
However, it is possible to send any command as passthrough (SG_IO), not only
ATA PASS-THROUGH (ATA-16 / ATA-12 commands).
So there will be a difference (by default) for SG_IO (passthrough) commands
that are not ATA PASS-THROUGH commands (ATA-16 / ATA-12 commands).
(E.g. if you send a regular SCSI read/write command via SG_IO to an ATA device,
and if that command generates sense data, the default sense data format would
be different.)
Is this a concern?
I have a feeling that some user space program that blindly assumes that the
sense data will be in fixed format (for e.g. a command that does an invalid
read) using SG_IO will start to complain because of a "regression".
Thus, perhaps it is safest to just drop this patch, and let users of
passthrough commands (SG_IO) simply learn how to parse sense data properly,
since there will/can always be someone complaining. My personal feeling is
that passthrough commands should simply follow the storage standard exactly,
and if a user space application does adhere to the standard, tough luck,
why are you using passthrough commands instead of regular commands then?
Passthrough commands by definition follow a specific storage standard,
and not the Linux kernel block layer API.
Kind regards,
Niklas
