scsi: hpsa: fix allocation size for scsi_host_alloc()

PATCH In review Alexey V. Vissarionov 4 participants 9 messages Jan 16, 2023 – Jan 19, 2023

Subsystems (2)

SCSI Hewlett-packard smart array RAID driver (hpsa)

lore

Share & tools

lore

Alexey V. Vissarionov

author

2023-01-16 13:38 UTC #0 lore raw

PATCH 1/1 scsi: hpsa: fix allocation size for scsi_host_alloc()

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 <[email protected]>

drivers/scsi/hpsa.c +1 -1

index 4dbf51e2623ad543..f6da34850af9dd46 100644
@@ -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;

Signature · 3 lines

Alexey V. Vissarionov
gremlin ПРИ altlinux ТЧК org; +vii-cmiii-ccxxix-lxxix-xlii
GPG: 0D92F19E1C0DC36E27F61A29CD17E2B43D879005 @ hkp://keys.gnupg.net

Bart Van Assche

2023-01-16 22:41 UTC #1 lore raw

Quoted 1 line · Alexey V. Vissarionov

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

author

2023-01-17 09:56 UTC #2 lore raw

Quoted 6 lines · Bart Van Assche

Quoted 2 lines
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?

Signature · 3 lines

Alexey V. Vissarionov
gremlin ПРИ altlinux ТЧК org; +vii-cmiii-ccxxix-lxxix-xlii
GPG: 0D92F19E1C0DC36E27F61A29CD17E2B43D879005 @ hkp://keys.gnupg.net

Alexey Khoroshilov

2023-01-17 10:39 UTC #3 lore raw

Quoted 9 lines · Alexey V. Vissarionov

Quoted 6 lines · Bart Van Assche
Quoted 2 lines
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.")

Quoted 5 lines

% 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

author

2023-01-17 23:34 UTC #4 lore raw

Quoted 11 lines · Alexey Khoroshilov

Quoted 11 lines
Quoted 6 lines
Quoted 2 lines
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?

Quoted 6 lines

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.

Signature · 3 lines

Alexey V. Vissarionov
gremlin ПРИ altlinux ТЧК org; +vii-cmiii-ccxxix-lxxix-xlii
GPG: 0D92F19E1C0DC36E27F61A29CD17E2B43D879005 @ hkp://keys.gnupg.net

Bart Van Assche

2023-01-17 23:26 UTC #5 lore raw

Quoted 1 line · Alexey V. Vissarionov

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.

Alexey V. Vissarionov

author

2023-01-18 03:13 UTC #6 lore raw

PATCH 1/1 scsi: hpsa: fix allocation size for scsi_host_alloc()

Quoted 2 lines · Bart Van Assche

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

ACK. Resending:

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 edd163687ea5 ("[SCSI] hpsa: combine hpsa_scsi_detect and hpsa_register_scsi")

Signed-off-by Alexey V. Vissarionov <[email protected]>

drivers/scsi/hpsa.c +1 -1

index 4dbf51e2623ad543..f6da34850af9dd46 100644
@@ -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;

Signature · 3 lines

Alexey V. Vissarionov
gremlin ПРИ altlinux ТЧК org; +vii-cmiii-ccxxix-lxxix-xlii
GPG: 0D92F19E1C0DC36E27F61A29CD17E2B43D879005 @ hkp://keys.gnupg.net

Martin K. Petersen

2023-01-19 00:15 UTC #7 lore raw

Alexey,

Next time please use -v2 instead of replying when you post a version 2 of a patch.

Quoted 6 lines · Bart Van Assche

Quoted 3 lines
My understanding is that you used an incorrect commit hash. Hence, it is up to you to fix the commit hash.
ACK. Resending:

Comments such as this ACK must go below a '---' separator to avoid ending up in the commit log.

And finally, the updated commit hash now no longer matches the commit description:

Quoted 1 line

Fixes edd163687ea5 ("[SCSI] hpsa: combine hpsa_scsi_detect and hpsa_register_scsi")

$ git log --oneline edd163687ea5 -1
edd163687ea5 [SCSI] hpsa: add driver for HP Smart Array controllers.

I fixed these up and applied to 6.2/scsi-fixes, thanks!

Signature · 1 line

Martin K. Petersen	Oracle Linux Engineering

Martin K. Petersen

Applied

2023-01-19 00:30 UTC #8 lore raw

In reply to Alexey V. Vissarionov

Quoted 8 lines · Alexey V. Vissarionov

Quoted 2 lines · Bart Van Assche
My understanding is that you used an incorrect commit hash. Hence, it is up to you to fix the commit hash.
ACK. Resending:
[...]

Applied to 6.2/scsi-fixes, thanks!

[1/1] scsi: hpsa: fix allocation size for scsi_host_alloc()

      https://git.kernel.org/mkp/scsi/c/bbbd25499100

Signature · 1 line

Martin K. Petersen	Oracle Linux Engineering