Linux SCSI subsystem development
 help / color / mirror / Atom feed
* [PATCH v2] scsi: alua: Use access state macros
@ 2026-08-05 14:59 John Garry
  2026-08-05 15:18 ` sashiko-bot
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: John Garry @ 2026-08-05 14:59 UTC (permalink / raw)
  To: James.Bottomley, martin.petersen
  Cc: linux-scsi, hare, bvanassche, John Garry, 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>
---
Differences to v1:
- use FIELD_GET() (Bart)

diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c
index 80ab0ff921d43..d81ec47a71402 100644
--- a/drivers/scsi/device_handler/scsi_dh_alua.c
+++ b/drivers/scsi/device_handler/scsi_dh_alua.c
@@ -5,6 +5,7 @@
  * Copyright (C) 2007-2010 Hannes Reinecke, SUSE Linux Products GmbH.
  * All rights reserved.
  */
+#include <linux/bitfield.h>
 #include <linux/slab.h>
 #include <linux/delay.h>
 #include <linux/module.h>
@@ -693,8 +694,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 = FIELD_GET(SCSI_ACCESS_STATE_MASK, desc[0]);
+					tmp_pg->pref = FIELD_GET(SCSI_ACCESS_STATE_PREFERRED, desc[0]);
 					rcu_read_lock();
 					list_for_each_entry_rcu(h,
 						&tmp_pg->dh_list, node) {
-- 
2.43.7


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] scsi: alua: Use access state macros
  2026-08-05 14:59 [PATCH v2] scsi: alua: Use access state macros John Garry
@ 2026-08-05 15:18 ` sashiko-bot
  2026-08-05 17:40 ` Bart Van Assche
  2026-08-07 15:22 ` Martin K. Petersen (Oracle)
  2 siblings, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-08-05 15:18 UTC (permalink / raw)
  To: John Garry; +Cc: linux-scsi

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

Pre-existing issues:
- [High] Out-of-bounds heap read when parsing Target Port Group descriptors from a malformed RTPG response.
--

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

scsi: alua: Use access state macros

This commit replaces hardcoded bitwise operations with SCSI access state
macros when parsing Target Port Group descriptors in alua_rtpg().

> diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c
> index 80ab0ff921d43..d81ec47a71402 100644
> --- a/drivers/scsi/device_handler/scsi_dh_alua.c
> +++ b/drivers/scsi/device_handler/scsi_dh_alua.c

[ ... ]

> @@ -693,8 +694,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 = FIELD_GET(SCSI_ACCESS_STATE_MASK, desc[0]);
> +					tmp_pg->pref = FIELD_GET(SCSI_ACCESS_STATE_PREFERRED, desc[0]);

[Severity: High]
This is a pre-existing issue, but is there a risk of an out-of-bounds heap
read on the desc pointer here?

Looking at alua_rtpg() in drivers/scsi/device_handler/scsi_dh_alua.c, the
loop iterating over the descriptors evaluates the length returned by the
target:

for (k = tpg_desc_tbl_off, desc = buff + tpg_desc_tbl_off;
     k < len;
     k += off, desc += off) {
        u16 group_id = get_unaligned_be16(&desc[2]);

If the target returns a malformed REPORT TARGET PORT GROUPS response where
len indicates a truncated final descriptor (for example, fewer than 8 bytes
remain), the loop condition k < len still evaluates to true.

Could this cause the reads of desc[0], desc[2], and desc[7] to access
memory past the end of the kmalloc buffer if a target provides a crafted
response length?

>  					rcu_read_lock();
>  					list_for_each_entry_rcu(h,
>  						&tmp_pg->dh_list, node) {

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] scsi: alua: Use access state macros
  2026-08-05 14:59 [PATCH v2] scsi: alua: Use access state macros John Garry
  2026-08-05 15:18 ` sashiko-bot
@ 2026-08-05 17:40 ` Bart Van Assche
  2026-08-07 15:22 ` Martin K. Petersen (Oracle)
  2 siblings, 0 replies; 4+ messages in thread
From: Bart Van Assche @ 2026-08-05 17:40 UTC (permalink / raw)
  To: John Garry, James.Bottomley, martin.petersen; +Cc: linux-scsi, hare, John Garry

On 8/5/26 7:59 AM, John Garry wrote:
> Use the SCSI access state macros in alua_rtpg(), which is better than
> hardcoding.

Reviewed-by: Bart Van Assche <bvanassche@acm.org>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] scsi: alua: Use access state macros
  2026-08-05 14:59 [PATCH v2] scsi: alua: Use access state macros John Garry
  2026-08-05 15:18 ` sashiko-bot
  2026-08-05 17:40 ` Bart Van Assche
@ 2026-08-07 15:22 ` Martin K. Petersen (Oracle)
  2 siblings, 0 replies; 4+ messages in thread
From: Martin K. Petersen (Oracle) @ 2026-08-07 15:22 UTC (permalink / raw)
  To: John Garry
  Cc: James.Bottomley, martin.petersen, linux-scsi, hare, bvanassche,
	John Garry


John,

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

Applied to 7.3/scsi-staging, thanks!

-- 
Martin K. Petersen

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-08-07 15:23 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-05 14:59 [PATCH v2] scsi: alua: Use access state macros John Garry
2026-08-05 15:18 ` sashiko-bot
2026-08-05 17:40 ` Bart Van Assche
2026-08-07 15:22 ` Martin K. Petersen (Oracle)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox