scsi: hpsa: fix allocation size for scsi_host_alloc()

Commit Message

Alexey V. Vissarionov Jan. 16, 2023, 1:31 p.m. UTC

The 'h' is a pointer to struct ctlr_info, so it's just 4 or 8
bytes, while the structure itself is much bigger.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Fixes: b705690d8d16f708 ("[SCSI] hpsa: combine hpsa_scsi_detect and hpsa_register_scsi")
Signed-off-by: Alexey V. Vissarionov <[e-mail]>

Comments

Bart Van Assche Jan. 16, 2023, 10:41 p.m. UTC | #1

On 1/16/23 05:31, Alexey V. Vissarionov wrote:
> Fixes: b705690d8d16f708 ("[SCSI] hpsa: combine hpsa_scsi_detect and hpsa_register_scsi")

That seems incorrect to me. Shouldn't the Fixes tag be changed into the 
following?

Fixes: edd163687ea5 ("[SCSI] hpsa: add driver for HP Smart Array 
controllers.")

Thanks,

Bart.

Alexey V. Vissarionov Jan. 17, 2023, 9:56 a.m. UTC | #2

On 2023-01-16 14:41:08 -0800, Bart Van Assche wrote:

 >> Fixes: b705690d8d16f708 ("[SCSI] hpsa: combine hpsa_scsi_detect
 >> and hpsa_register_scsi")
 > That seems incorrect to me. Shouldn't the Fixes tag be changed
 > into the following?
 > Fixes: edd163687ea5 ("[SCSI] hpsa: add driver for HP Smart Array
 > controllers.")

% git blame -L 5853,+1 -- drivers/scsi/hpsa.c
b705690d8d16f7081 (Stephen M. Cameron 2012-01-19 14:00:53 -0600 5853)
	sh = scsi_host_alloc(&hpsa_driver_template, sizeof(h));

Is anything wrong here?

Alexey Khoroshilov Jan. 17, 2023, 10:35 a.m. UTC | #3

On 17.01.2023 12:56, Alexey V. Vissarionov wrote:
> On 2023-01-16 14:41:08 -0800, Bart Van Assche wrote:
> 
>  >> Fixes: b705690d8d16f708 ("[SCSI] hpsa: combine hpsa_scsi_detect
>  >> and hpsa_register_scsi")
>  > That seems incorrect to me. Shouldn't the Fixes tag be changed
>  > into the following?
>  > Fixes: edd163687ea5 ("[SCSI] hpsa: add driver for HP Smart Array
>  > controllers.")
> 
> % git blame -L 5853,+1 -- drivers/scsi/hpsa.c
> b705690d8d16f7081 (Stephen M. Cameron 2012-01-19 14:00:53 -0600 5853)
> 	sh = scsi_host_alloc(&hpsa_driver_template, sizeof(h));
> 
> Is anything wrong here?

Yes, this commit just moves

  sh = scsi_host_alloc(&hpsa_driver_template, sizeof(h));

from one function to another one.


edd163687ea5 ("[SCSI] hpsa: add driver for HP Smart Array controllers.")
is the correct commit introducing the bug.

--
Alexey

Alexey V. Vissarionov Jan. 17, 2023, 9:12 p.m. UTC | #4

On 2023-01-17 13:35:54 +0300, Alexey Khoroshilov wrote:

 >>>> Fixes: b705690d8d16f708 ("[SCSI] hpsa: combine
 >>>> hpsa_scsi_detect and hpsa_register_scsi")
 >>> That seems incorrect to me. Shouldn't the Fixes tag be
 >>> changed into the following?
 >>> Fixes: edd163687ea5 ("[SCSI] hpsa: add driver for HP
 >>> Smart Array controllers.")
 >> % git blame -L 5853,+1 -- drivers/scsi/hpsa.c
 >> b705690d8d16f7081 (Stephen M. Cameron 2012-01-19 14:00:53
 >> -0600 5853)
 >> sh = scsi_host_alloc(&hpsa_driver_template, sizeof(h));
 >> Is anything wrong here?
 > Yes, this commit just moves
 > sh = scsi_host_alloc(&hpsa_driver_template, sizeof(h));
 > from one function to another one.
 > edd163687ea5 ("[SCSI] hpsa: add driver for HP Smart Array
 > controllers.")
 > is the correct commit introducing the bug.

ACK.

2 BVA: please use the correct commit number.

Bart Van Assche Jan. 17, 2023, 9:23 p.m. UTC | #5

On 1/17/23 13:12, Alexey V. Vissarionov wrote:
> 2 BVA: please use the correct commit number.

???

My understanding is that you used an incorrect commit hash. Hence, it is 
up to you to fix the commit hash.

Did I perhaps misunderstand something?

Bart.

Patch

diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index 4dbf51e2623ad543..f6da34850af9dd46 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -5850,7 +5850,7 @@  static int hpsa_scsi_host_alloc(struct ctlr_info *h)
 {
 	struct Scsi_Host *sh;
 
-	sh = scsi_host_alloc(&hpsa_driver_template, sizeof(h));
+	sh = scsi_host_alloc(&hpsa_driver_template, sizeof(struct ctlr_info));
 	if (sh == NULL) {
 		dev_err(&h->pdev->dev, "scsi_host_alloc failed\n");
 		return -ENOMEM;