linux-kernel - Re: [PATCH] scsi: megaraid_sas: struct MR_HOST_DEVICE_LIST: Replace 1-element array with flexible array

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: <a9632309e6b5df95c7359ec9623bb1b95f764eb4.camel@HansenPartnership.com>
Date: Thu, 11 Jul 2024 11:11:31 -0700
From: James Bottomley <James.Bottomley@...senPartnership.com>
To: Kees Cook <kees@...nel.org>, Kashyap Desai <kashyap.desai@...adcom.com>
Cc: Sumit Saxena <sumit.saxena@...adcom.com>, Shivasharan S
	 <shivasharan.srikanteshwara@...adcom.com>, Chandrakanth patil
	 <chandrakanth.patil@...adcom.com>, "Martin K. Petersen"
	 <martin.petersen@...cle.com>, megaraidlinux.pdl@...adcom.com, 
	linux-scsi@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-hardening@...r.kernel.org
Subject: Re: [PATCH] scsi: megaraid_sas: struct MR_HOST_DEVICE_LIST: Replace
 1-element array with flexible array

On Thu, 2024-07-11 at 08:58 -0700, Kees Cook wrote:
> Replace the deprecated[1] use of a 1-element array in
> struct MR_HOST_DEVICE_LIST with a modern flexible array.
> 
> One binary difference appears in megasas_host_device_list_query():
> 
>         struct MR_HOST_DEVICE_LIST *ci;
>         ...
>         ci = instance->host_device_list_buf;
>         ...
>         memset(ci, 0, sizeof(*ci));
> 
> The memset() clears only the non-flexible array fields. Looking at
> the rest of the function, this appears to be fine: firmware is using
> this region to communicate with the kernel, so it likely never made
> sense to clear the first MR_HOST_DEVICE_LIST_ENTRY.

That's not necessarily a safe assumption: older qlogic for instance
uses zeroing an entry to stop the card mailbox processing.  Looking at
the driver, I think you're right: it's only used for card to host
communication, so clearing it is irrelevant, but it could be relevant
if it were also used for host to card communication.

Regards,

James