Re: [PATCH v3 34/40] target: 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] When Unit Attention Interlock Control (`emulate_ua_intlck_ctrl`) is configured to not clear UAs, reporting a Unit Attention fails and results in a permanent `SAM_STAT_BUSY` loop instead of sending a `CHECK_CONDITION`.
--

commit 38089f6eddcf2270af001616b6a77553df3408ee
Author: Damien Le Moal <dlemoal@kernel.org>

target: use 16-bits defined sense codes

Refactor the target core code to use the 16-bits sense codes for building
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. This helps with code clarity as the sense codes
being processed are easier to test and self-documented.

No functional change intended.

> diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
> index dcfe945949167..ba208def8cb59 100644
> --- a/drivers/target/target_core_transport.c
> +++ b/drivers/target/target_core_transport.c

[ ... ]

> @@ -3566,21 +3567,19 @@ static void translate_sense_reason(struct se_cmd *cmd, sense_reason_t reason)
>  
>  	key = sd->key;
>  	if (reason == TCM_CHECK_CONDITION_UNIT_ATTENTION) {
> -		if (!core_scsi3_ua_for_check_condition(cmd, &key, &asc,
> -						       &ascq)) {
> +		if (!core_scsi3_ua_for_check_condition(cmd, &key, &code)) {
>  			cmd->scsi_status = SAM_STAT_BUSY;
>  			return;
>  		}
>  	} else {

[ ... ]

> diff --git a/drivers/target/target_core_ua.c b/drivers/target/target_core_ua.c
> index 4276690fb6cbd..ea97b05d47ab7 100644
> --- a/drivers/target/target_core_ua.c
> +++ b/drivers/target/target_core_ua.c

[ ... ]

> @@ -232,8 +230,7 @@ bool core_scsi3_ua_for_check_condition(struct se_cmd *cmd, u8 *key, u8 *asc,
>  		 * clearing it.
>  		 */
>  		if (!dev_ua_intlck_clear) {
> -			*asc = ua->ua_asc;
> -			*ascq = ua->ua_ascq;
> +			*code = scsi_sense_code(ua->ua_asc, ua->ua_ascq);
>  			break;
>  		}

[Severity: High]
This is a pre-existing issue, but I noticed a logic error in the UA reporting
mechanism that is still present.

When core_scsi3_ua_for_check_condition() executes the `!dev_ua_intlck_clear`
branch shown above, it breaks out of the loop early without setting the local
variable `head = 0`. Because the function simply returns `head == 0` at the end,
it will always return false in this case.

As a result, when called by translate_sense_reason(), this false return value
causes the target to respond with SAM_STAT_BUSY instead of building the proper
CHECK_CONDITION sense buffer.

Could this lead to an infinite retry loop? If the target responds with BUSY,
the initiator might indefinitely retry the command since the Unit Attention is
never cleared or properly reported, potentially making the LUN completely
inaccessible.

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