Re: [PATCH 12/37] scsi: st: use 16-bits defined sense codes

[Laurence Oberman <loberman@redhat.com>]

On Mon, 2026-08-31 at 02:19 +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider:
> - [Medium] Incorrectly assigning ASC value to ASCQ in SCSI
> tracepoint.
> --
> 
> commit c1f7c0aa9501c3562bd45de8a85431d940fa338b
> Author: Damien Le Moal <dlemoal@kernel.org>
> Subject: scsi: st: use 16-bits defined sense codes
> 
> This commit refactors the SCSI tape driver to use the 16-bit
> sense_code
> field of struct scsi_sense_hdr. It replaces hard-coded additional
> sense
> codes and qualifiers with enum values defined in
> include/scsi/scsi_sense.h
> to improve code clarity.
> 
> While reviewing this patch series, I noticed an issue introduced in
> the
> related change to include/trace/events/scsi.h:
> 
> > diff --git a/include/trace/events/scsi.h
> > b/include/trace/events/scsi.h
> > --- a/include/trace/events/scsi.h
> > +++ b/include/trace/events/scsi.h
> > @@ -303,8 +303,8 @@
> > DECLARE_EVENT_CLASS(scsi_cmd_done_timeout_template,
> >  		if (cmd->sense_buffer && SCSI_SENSE_VALID(cmd) &&
> >  		    scsi_command_normalize_sense(cmd, &sshdr)) {
> >  			__entry->sense_key = sshdr.sense_key;
> > -			__entry->asc = sshdr.asc;
> > -			__entry->ascq = sshdr.ascq;
> > +			__entry->asc = scsi_sense_asc(&sshdr);
> > +			__entry->ascq = scsi_sense_asc(&sshdr); <-
> > --- here
>                                         ^^^^^^^^^^^^^^
> 
> [Severity: Medium]
> Is there a typo in this assignment?
> 
> In the scsi_cmd_done_timeout_template tracepoint, __entry->ascq is
> being
> assigned scsi_sense_asc(&sshdr) instead of the ASCQ value. This
> appears to
> log the Additional Sense Code for both the asc and ascq fields, which
> will
> result in lost diagnostic data when userspace tools analyze SCSI
> errors via
> tracepoints.
> 
> Should this use scsi_sense_ascq(&sshdr) for the ascq field instead?

Hello Damien
This seems to be real issue here that sashiko found

We assign the same for __entry->ascq  as we do for __entry->asc i.e.
scsi_sense_asc(&sshdr)

We need to have it as

+			__entry->asc = scsi_sense_asc(&sshdr);
+			__entry->ascq = scsi_sense_ascq(&sshdr); <--

Otherwise we will miss the ASCQ

Thanks
Laurence