[PATCH] scsi: alua: Use access state macros

[PATCH] scsi: alua: Use access state macros

[John Garry]

From: John Garry <john.garry@linux.dev>

Use the SCSI access state macros in alua_rtpg(), which is better than
hardcoding.

Signed-off-by: John Garry <john.g.garry@oracle.com>

diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c
index 80ab0ff921d43..4258c48d93339 100644
--- a/drivers/scsi/device_handler/scsi_dh_alua.c
+++ b/drivers/scsi/device_handler/scsi_dh_alua.c
@@ -693,8 +693,8 @@ static int alua_rtpg(struct scsi_device *sdev, struct alua_port_group *pg)
 				    !(tmp_pg->flags & ALUA_PG_RUNNING)) {
 					struct alua_dh_data *h;
 
-					tmp_pg->state = desc[0] & 0x0f;
-					tmp_pg->pref = desc[0] >> 7;
+					tmp_pg->state = desc[0] & SCSI_ACCESS_STATE_MASK;
+					tmp_pg->pref = desc[0] >> __fls(SCSI_ACCESS_STATE_PREFERRED);
 					rcu_read_lock();
 					list_for_each_entry_rcu(h,
 						&tmp_pg->dh_list, node) {
-- 
2.43.7

Re: [PATCH] scsi: alua: Use access state macros

[sashiko-bot]

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

Pre-existing issues:
- [High] Uninitialized kernel heap memory parsing due to missing zero-initialization on reallocation.
- [High] Heap out-of-bounds read in the ALUA RTPG descriptor parsing loop due to missing bounds check.
--

--- Patch [1]: [PATCH] scsi: alua: Use access state macros ---
commit 2dc602acd70bdca60f3657da1a036b1c0fed56d1
Author: John Garry <john.garry@linux.dev>

scsi: alua: Use access state macros

Use the SCSI access state macros in alua_rtpg(), which is better than
hardcoding.

> diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c
> index 80ab0ff921d43..4258c48d93339 100644
> --- a/drivers/scsi/device_handler/scsi_dh_alua.c
> +++ b/drivers/scsi/device_handler/scsi_dh_alua.c
> @@ -693,8 +693,8 @@ static int alua_rtpg(struct scsi_device *sdev, struct alua_port_group *pg)

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing alua_rtpg(),
I noticed a potential issue with the buffer reallocation earlier in the
function:

    buff = kmalloc(bufflen, GFP_KERNEL);

If the subsequent submit_rtpg() succeeds but transfers fewer bytes than
requested (a short read), wouldn't the trailing portion of the buffer remain
uninitialized? Since the function parses up to the reported len, could this
cause it to parse uninitialized kernel heap memory if a malicious device
advertises a large length but returns a short response?

[Severity: High]
This is also a pre-existing issue, but looking at the descriptor parsing loop
just before the modified lines:

    for (k = tpg_desc_tbl_off, desc = buff + tpg_desc_tbl_off;
         k < len;
         k += off, desc += off) {
        ...
        off = 8 + (desc[7] * 4);
    }

Does the loop condition k < len ensure that a full 8-byte descriptor header
is available? If a device returns a truncated response where len - k < 8,
wouldn't the accesses to desc[0] through desc[7] read past the end of the heap
allocation?

>  				    !(tmp_pg->flags & ALUA_PG_RUNNING)) {
>  					struct alua_dh_data *h;
>  
> -					tmp_pg->state = desc[0] & 0x0f;
> -					tmp_pg->pref = desc[0] >> 7;
> +					tmp_pg->state = desc[0] & SCSI_ACCESS_STATE_MASK;
> +					tmp_pg->pref = desc[0] >> __fls(SCSI_ACCESS_STATE_PREFERRED);
>  					rcu_read_lock();
>  					list_for_each_entry_rcu(h,
>  						&tmp_pg->dh_list, node) {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260804111615.2368927-1-john.g.garry@oracle.com?part=1

Re: [PATCH] scsi: alua: Use access state macros

[Bart Van Assche]

On 8/4/26 4:16 AM, John Garry wrote:
> diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c
> index 80ab0ff921d43..4258c48d93339 100644
> --- a/drivers/scsi/device_handler/scsi_dh_alua.c
> +++ b/drivers/scsi/device_handler/scsi_dh_alua.c
> @@ -693,8 +693,8 @@ static int alua_rtpg(struct scsi_device *sdev, struct alua_port_group *pg)
>   				    !(tmp_pg->flags & ALUA_PG_RUNNING)) {
>   					struct alua_dh_data *h;
>   
> -					tmp_pg->state = desc[0] & 0x0f;
> -					tmp_pg->pref = desc[0] >> 7;
> +					tmp_pg->state = desc[0] & SCSI_ACCESS_STATE_MASK;
> +					tmp_pg->pref = desc[0] >> __fls(SCSI_ACCESS_STATE_PREFERRED);
>   					rcu_read_lock();
>   					list_for_each_entry_rcu(h,
>   						&tmp_pg->dh_list, node) {

Please use FIELD_GET() instead of open-coding this macro.

Thanks,

Bart.