[PATCH] scsi: mpt3sas: Convert to flexible arrays

[PATCH] scsi: mpt3sas: Convert to flexible arrays

[Kees Cook]

This converts to a flexible array instead of the old-style 1-element
arrays. The existing code already did the correct math for finding the
size of the resulting flexible array structure, so there is no binary
difference.

The other two structures converted to use flexible arrays appear to
have no users at all.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 drivers/scsi/mpt3sas/mpi/mpi2_ioc.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/mpt3sas/mpi/mpi2_ioc.h b/drivers/scsi/mpt3sas/mpi/mpi2_ioc.h
index e83c7c529dc9..2c57115172cf 100644
--- a/drivers/scsi/mpt3sas/mpi/mpi2_ioc.h
+++ b/drivers/scsi/mpt3sas/mpi/mpi2_ioc.h
@@ -537,7 +537,7 @@ typedef struct _MPI2_EVENT_NOTIFICATION_REPLY {
 	U16 Event;		/*0x14 */
 	U16 Reserved4;		/*0x16 */
 	U32 EventContext;	/*0x18 */
-	U32 EventData[1];	/*0x1C */
+	U32 EventData[];	/*0x1C */
 } MPI2_EVENT_NOTIFICATION_REPLY, *PTR_MPI2_EVENT_NOTIFICATION_REPLY,
 	Mpi2EventNotificationReply_t,
 	*pMpi2EventNotificationReply_t;
@@ -639,7 +639,7 @@ typedef struct _MPI2_EVENT_DATA_HOST_MESSAGE {
 	U8 Reserved1;		/*0x01 */
 	U16 Reserved2;		/*0x02 */
 	U32 Reserved3;		/*0x04 */
-	U32 HostData[1];	/*0x08 */
+	U32 HostData[];		/*0x08 */
 } MPI2_EVENT_DATA_HOST_MESSAGE, *PTR_MPI2_EVENT_DATA_HOST_MESSAGE,
 	Mpi2EventDataHostMessage_t, *pMpi2EventDataHostMessage_t;
 
@@ -1397,7 +1397,7 @@ typedef struct _MPI2_SEND_HOST_MESSAGE_REQUEST {
 	U32 Reserved8;		/*0x18 */
 	U32 Reserved9;		/*0x1C */
 	U32 Reserved10;		/*0x20 */
-	U32 HostData[1];	/*0x24 */
+	U32 HostData[];		/*0x24 */
 } MPI2_SEND_HOST_MESSAGE_REQUEST,
 	*PTR_MPI2_SEND_HOST_MESSAGE_REQUEST,
 	Mpi2SendHostMessageRequest_t,
-- 
2.30.2

Re: [PATCH] scsi: mpt3sas: Convert to flexible arrays

[Martin K. Petersen]

Kees,

> This converts to a flexible array instead of the old-style 1-element
> arrays. The existing code already did the correct math for finding the
> size of the resulting flexible array structure, so there is no binary
> difference.

Applied to 5.18/scsi-staging, thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH] scsi: mpt3sas: Convert to flexible arrays

[Martin K. Petersen]

On Tue, 1 Feb 2022 14:39:48 -0800, Kees Cook wrote:

> This converts to a flexible array instead of the old-style 1-element
> arrays. The existing code already did the correct math for finding the
> size of the resulting flexible array structure, so there is no binary
> difference.
> 
> The other two structures converted to use flexible arrays appear to
> have no users at all.
> 
> [...]

Applied to 5.18/scsi-queue, thanks!

[1/1] scsi: mpt3sas: Convert to flexible arrays
      https://git.kernel.org/mkp/scsi/c/d20b3dae630f

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH] scsi: mpt3sas: Convert to flexible arrays

[Sreekanth Reddy]

[-- Attachment #1: Type: text/plain, Size: 4200 bytes --]

Hi,

I am observing below kernel panic when I load the driver with this
patch changes. After reverting this patch changes then the driver is
getting loaded successfully.

[  414.493579] mpt3sas_cm0: port enable: SUCCESS
[  414.498313] BUG: unable to handle page fault for address: 00000024000000ba
[  414.505182] #PF: supervisor read access in kernel mode
[  414.510322] #PF: error_code(0x0000) - not-present page
[  414.515462] PGD 0 P4D 0
[  414.518000] Oops: 0000 [#1] PREEMPT SMP NOPTI
[  414.522360] CPU: 31 PID: 48 Comm: kworker/u98:0 Kdump: loaded
Tainted: G           OE     5.17.0-rc1+ #1
[  414.531833] Hardware name: Supermicro AS -1114S-WTRT/H12SSW-NT,
BIOS 1.1a 05/28/2020
[  414.539564] Workqueue: events_unbound async_run_entry_fn
[  414.544877] RIP: 0010:scsih_qcmd+0x29/0x450 [mpt3sas]
[  414.549938] Code: 00 0f 1f 44 00 00 41 57 41 56 41 55 41 54 55 48
89 fd 53 48 89 f3 48 83 ec 18 f6 87 91 08 00 00 40 0f 85 e3 00 05
[  414.568684] RSP: 0018:ffffa397c67df8d0 EFLAGS: 00010246
[  414.573912] RAX: 0000002400000012 RBX: ffff947666d84208 RCX: ffff947666d84208
[  414.581042] RDX: ffff947603afa000 RSI: ffff947666d84208 RDI: ffff947603afa000
[  414.588168] RBP: ffff947603afa000 R08: 0000000000000020 R09: ffff94764ad2fa30
[  414.595301] R10: 0000000000000000 R11: 0000000000000001 R12: ffff947603afa000
[  414.602434] R13: ffff947667d87000 R14: ffff947603afa000 R15: ffffa397c67df9d0
[  414.609568] FS:  0000000000000000(0000) GS:ffff947d2fbc0000(0000)
knlGS:0000000000000000
[  414.617654] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  414.623399] CR2: 00000024000000ba CR3: 00000004c9e0a000 CR4: 0000000000350ee0
[  414.630531] Call Trace:
[  414.632977]  <TASK>
[  414.635084]  scsi_queue_rq+0x36e/0xa70
[  414.638836]  blk_mq_dispatch_rq_list+0x2f6/0x8e0
[  414.643455]  ? get_page_from_freelist+0xb70/0xe50
[  414.648161]  ? __sbitmap_get_word+0x37/0x80
[  414.652347]  __blk_mq_sched_dispatch_requests+0xb2/0x140
[  414.657661]  blk_mq_sched_dispatch_requests+0x30/0x60
[  414.662712]  __blk_mq_run_hw_queue+0x34/0x90
[  414.666985]  __blk_mq_delay_run_hw_queue+0x18b/0x1c0
[  414.671949]  blk_mq_sched_insert_request+0xd3/0x140
[  414.676830]  blk_execute_rq+0x53/0xf0
[  414.680495]  __scsi_execute+0x101/0x260
[  414.684337]  scsi_probe_and_add_lun+0x181/0xe10
[  414.688869]  __scsi_scan_target+0xec/0x5a0
[  414.692969]  ? _raw_spin_unlock_irqrestore+0x1f/0x31
[  414.697933]  ? __pm_runtime_resume+0x54/0x70
[  414.702208]  scsi_scan_target+0xf8/0x110
[  414.706135]  sas_rphy_add+0x162/0x1b0 [scsi_transport_sas]
[  414.711620]  mpt3sas_transport_port_add+0x315/0x480 [mpt3sas]
[  414.717375]  _scsih_complete_devices_scanning+0x2f8/0x336 [mpt3sas]
[  414.723648]  scsih_scan_finished.cold.103+0x91/0xe3 [mpt3sas]
[  414.729393]  do_scsi_scan_host+0x6f/0x90
[  414.733321]  do_scan_async+0x17/0x150
[  414.736987]  async_run_entry_fn+0x30/0x130
[  414.741085]  process_one_work+0x1c8/0x390
[  414.745099]  worker_thread+0x30/0x350
[  414.748764]  ? process_one_work+0x390/0x390
[  414.752951]  kthread+0xe8/0x110
[  414.756098]  ? kthread_complete_and_exit+0x20/0x20
[  414.760891]  ret_from_fork+0x22/0x30
[  414.764469]  </TASK>
[  414.766662] Modules linked in: mpt3sas(OE) xt_CHECKSUM
xt_MASQUERADE xt_conntrack ipt_REJECT nf_nat_tftp nft_objref
nf_conntrack_tc
[  414.766715]  drm_ttm_helper ttm ahci crc32c_intel libahci drm
bnxt_en libata raid_class scsi_transport_sas pinctrl_amd dm_mirror d]
[  414.869470] CR2: 00000024000000ba

Thanks,
Sreekanth

On Sat, Feb 12, 2022 at 4:55 AM Martin K. Petersen
<martin.petersen@oracle.com> wrote:
>
> On Tue, 1 Feb 2022 14:39:48 -0800, Kees Cook wrote:
>
> > This converts to a flexible array instead of the old-style 1-element
> > arrays. The existing code already did the correct math for finding the
> > size of the resulting flexible array structure, so there is no binary
> > difference.
> >
> > The other two structures converted to use flexible arrays appear to
> > have no users at all.
> >
> > [...]
>
> Applied to 5.18/scsi-queue, thanks!
>
> [1/1] scsi: mpt3sas: Convert to flexible arrays
>       https://git.kernel.org/mkp/scsi/c/d20b3dae630f
>
> --
> Martin K. Petersen      Oracle Linux Engineering

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4218 bytes --]

Re: [PATCH] scsi: mpt3sas: Convert to flexible arrays

[Martin K. Petersen]

Sreekanth,

> I am observing below kernel panic when I load the driver with this
> patch changes. After reverting this patch changes then the driver is
> getting loaded successfully.

I am puzzled. The driver loads fine for me. I have verified that the
generated object file is identical before and after Kees' patch.

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH] scsi: mpt3sas: Convert to flexible arrays

[Sreekanth Reddy]

[-- Attachment #1: Type: text/plain, Size: 662 bytes --]

On Wed, Mar 9, 2022 at 8:50 AM Martin K. Petersen
<martin.petersen@oracle.com> wrote:
>
>
> Sreekanth,
>
> > I am observing below kernel panic when I load the driver with this
> > patch changes. After reverting this patch changes then the driver is
> > getting loaded successfully.
>
> I am puzzled. The driver loads fine for me. I have verified that the
> generated object file is identical before and after Kees' patch.

Looks like we will observe this issue only if DIF Type 2 drive is
connected to the HBA. When this panic occurred I had connected one DIF
Type2 drive to the HBA.

Thanks,
Sreekanth

>
> --
> Martin K. Petersen      Oracle Linux Engineering

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4218 bytes --]

Re: [PATCH] scsi: mpt3sas: Convert to flexible arrays

[Martin K. Petersen]

Sreekanth,

> Looks like we will observe this issue only if DIF Type 2 drive is
> connected to the HBA. When this panic occurred I had connected one DIF
> Type2 drive to the HBA.

No problems with Type 2 devices observed here.

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH] scsi: mpt3sas: Convert to flexible arrays

[Sreekanth Reddy]

[-- Attachment #1: Type: text/plain, Size: 588 bytes --]

On Tue, Mar 15, 2022 at 8:58 AM Martin K. Petersen
<martin.petersen@oracle.com> wrote:
>
>
> Sreekanth,
>
> > Looks like we will observe this issue only if DIF Type 2 drive is
> > connected to the HBA. When this panic occurred I had connected one DIF
> > Type2 drive to the HBA.
>
> No problems with Type 2 devices observed here.

Martin,

Sorry, somehow I am also not able to reproduce the issue again. I see
that the driver is getting loaded successfully. I have to recollect
what config I have used earlier.

Thanks,
Sreekanth
>
> --
> Martin K. Petersen      Oracle Linux Engineering

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4218 bytes --]

Re: [PATCH] scsi: mpt3sas: Convert to flexible arrays

[Kees Cook]

On Tue, Mar 08, 2022 at 10:20:28PM -0500, Martin K. Petersen wrote:
> 
> Sreekanth,
> 
> > I am observing below kernel panic when I load the driver with this
> > patch changes. After reverting this patch changes then the driver is
> > getting loaded successfully.
> 
> I am puzzled. The driver loads fine for me. I have verified that the
> generated object file is identical before and after Kees' patch.

I've double-checked this again myself; I don't see any binary
difference. Can you share your .config? I was using defconfig with
these added:

SCSI_LOWLEVEL=y
SCSI_MPT3SAS=y

-- 
Kees Cook