Re: [PATCH v6 25/40] scsi: ps3rom: use 16-bits defined sense codes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Pre-existing cmd->result overwrite clobbers CHECK_CONDITION status
--

commit 4d82cf634e0fe3033c43713286bcedaf5b5d7dae
Author: Damien Le Moal <dlemoal@kernel.org>

scsi: ps3rom: use 16-bits defined sense codes

Refactor the PS3 DVD driver to use scsi_set_sense() and replace all
hard-coded additional sense codes and additional sense code qualifiers
with the enum values defined in include/scsi/scsi_sense.h.

> diff --git a/drivers/scsi/ps3rom.c b/drivers/scsi/ps3rom.c
> index a9c727d229313..c63c581b493c3 100644
> --- a/drivers/scsi/ps3rom.c
> +++ b/drivers/scsi/ps3rom.c
> @@ -233,7 +233,8 @@ static enum scsi_qc_status ps3rom_queuecommand_lck(struct scsi_cmnd *cmd)
>  	}
>  
>  	if (res) {
> -		scsi_build_sense(cmd, 0, ILLEGAL_REQUEST, 0, 0);
> +		scsi_set_sense(cmd, 0, ILLEGAL_REQUEST,
> +			       NO_ADDITIONAL_SENSE_INFORMATION);
>  		cmd->result = res;

[Severity: High]
This is a pre-existing issue, but does this cmd->result assignment overwrite
the status byte set by the sense functions?

Looking at scsi_set_sense() in scsi_lib.c, it sets the result:

void scsi_set_sense(struct scsi_cmnd *scmd, int desc, u8 key, u16 code)
{
        scsi_set_sense_buffer(desc, scmd->sense_buffer, key, code);
        scmd->result = SAM_STAT_CHECK_CONDITION;
}

Immediately assigning cmd->result = res in ps3rom_queuecommand_lck()
clobbers this SAM_STAT_CHECK_CONDITION status, which could bypass midlayer
sense processing on failed IO requests. Could this assignment be updated
so the midlayer properly sees the check condition?

>  		priv->curr_cmd = NULL;
>  		scsi_done(cmd);
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908090308.1085097-1-dlemoal@kernel.org?part=25