Re: [PATCH] scsi: alua: Use access state macros
[email protected] Tue, 04 Aug 2026 11:30:45 +0000
| Newsgroups | gmane.linux.scsi |
|---|---|
| Message-ID | <[email protected]> |
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-initi= alization on reallocation. - [High] Heap out-of-bounds read in the ALUA RTPG descriptor parsing loop d= ue to missing bounds check. -- --- Patch [1]: [PATCH] scsi: alua: Use access state macros --- commit 2dc602acd70bdca60f3657da1a036b1c0fed56d1 Author: John Garry <[email protected]> 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/de= vice_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 =3D 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 lo= op just before the modified lines: for (k =3D tpg_desc_tbl_off, desc =3D buff + tpg_desc_tbl_off; k < len; k +=3D off, desc +=3D off) { ... off =3D 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 h= eap allocation? > !(tmp_pg->flags & ALUA_PG_RUNNING)) { > struct alua_dh_data *h; > =20 > - tmp_pg->state =3D desc[0] & 0x0f; > - tmp_pg->pref =3D desc[0] >> 7; > + tmp_pg->state =3D desc[0] & SCSI_ACCESS_STATE_MASK; > + tmp_pg->pref =3D desc[0] >> __fls(SCSI_ACCESS_STATE_PREFERRED); > rcu_read_lock(); > list_for_each_entry_rcu(h, > &tmp_pg->dh_list, node) { --=20 Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260804111615.2368= [email protected]?part=3D1