linux-hardening - Re: [PATCH RFC] scsi: qla2xxx: zero default_item last in qla24xx_free_purex

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <e58d743b-a999-4e00-8f2e-31707744c5bb@embeddedor.com>
Date: Thu, 25 Sep 2025 19:22:56 +0200
From: "Gustavo A. R. Silva" <gustavo@...eddedor.com>
To: Bryan Gurney <bgurney@...hat.com>, jmeneghi@...hat.com,
 linux-nvme@...ts.infradead.org, kbusch@...nel.org, hch@....de,
 sagi@...mberg.me, axboe@...nel.dk
Cc: james.smart@...adcom.com, njavali@...vell.com,
 linux-scsi@...r.kernel.org, hare@...e.de, linux-hardening@...r.kernel.org,
 kees@...nel.org, gustavoars@...nel.org, emilne@...hat.com
Subject: Re: [PATCH RFC] scsi: qla2xxx: zero default_item last in
 qla24xx_free_purex_item

On 9/25/25 19:02, Bryan Gurney wrote:
> In order to avoid a null pointer dereference, the vha->default_item
> should be set to 0 last if the item pointer passed to the function
> matches.
> 
> BUG: kernel NULL pointer dereference, address: 0000000000000936
> ...
> RIP: 0010:qla24xx_free_purex_item+0x5e/0x90 [qla2xxx]
> ...
> Call Trace:
>   <TASK>
>   qla24xx_process_purex_list+0xda/0x110 [qla2xxx]
>   qla2x00_do_dpc+0x8ac/0xab0 [qla2xxx]
>   ? __pfx_qla2x00_do_dpc+0x10/0x10 [qla2xxx]
>   kthread+0xf9/0x240
>   ? __pfx_kthread+0x10/0x10
>   ret_from_fork+0xf1/0x110
>   ? __pfx_kthread+0x10/0x10
> 
> Also use a local variable to avoid multiple de-referencing of the item.
> 
> Fixes: 6f4b10226b6b ("scsi: qla2xxx: Fix memcpy() field-spanning write issue")
> Signed-off-by: Bryan Gurney <bgurney@...hat.com>
> ---
>   drivers/scsi/qla2xxx/qla_os.c | 8 +++++---
>   1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
> index 98a5c105fdfd..7e28c7e9aa60 100644
> --- a/drivers/scsi/qla2xxx/qla_os.c
> +++ b/drivers/scsi/qla2xxx/qla_os.c
> @@ -6459,9 +6459,11 @@ void qla24xx_process_purex_rdp(struct scsi_qla_host *vha,
>   void
>   qla24xx_free_purex_item(struct purex_item *item)
>   {
> -	if (item == &item->vha->default_item) {
> -		memset(&item->vha->default_item, 0, sizeof(struct purex_item));
> -		memset(&item->vha->__default_item_iocb, 0, QLA_DEFAULT_PAYLOAD_SIZE);
> +	scsi_qla_host_t *base_vha = item->vha;
> +
> +	if (item == &base_vha->default_item) {
> +		memset(&base_vha->__default_item_iocb, 0, QLA_DEFAULT_PAYLOAD_SIZE);
> +		memset(&base_vha->default_item, 0, sizeof(struct purex_item));
>   	} else
>   		kfree(item);
>   }

I see. I think it's probably better to go ahead with the revert, and then apply
the patch I proposed in my previous e-mail (it's more straightforward and introduces
fewer changes).

If you agree with that, I can submit both the revert and the patch.

Thanks
-Gustavo