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

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

[Alexey V. Vissarionov]

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

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 <gremlin@altlinux.org>

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;


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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH] scsi: hpsa: fix allocation size for scsi_host_alloc()

[Bart Van Assche]

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.

Re: [PATCH] scsi: hpsa: fix allocation size for scsi_host_alloc()

[Alexey V. Vissarionov]

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 V. Vissarionov
gremlin ПРИ altlinux ТЧК org; +vii-cmiii-ccxxix-lxxix-xlii
GPG: 0D92F19E1C0DC36E27F61A29CD17E2B43D879005 @ hkp://keys.gnupg.net

Re: [lvc-project] [PATCH] scsi: hpsa: fix allocation size for scsi_host_alloc()

[Alexey Khoroshilov]

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

Re: [lvc-project] [PATCH] scsi: hpsa: fix allocation size for scsi_host_alloc()

[Alexey V. Vissarionov]

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

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.


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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [lvc-project] [PATCH] scsi: hpsa: fix allocation size for scsi_host_alloc()

[Bart Van Assche]

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] scsi: hpsa: fix allocation size for scsi_host_alloc()

[Alexey V. Vissarionov]

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

On 2023-01-17 13:23:19 -0800, Bart Van Assche wrote:

 > 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 <gremlin@altlinux.org>

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;


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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH] scsi: hpsa: fix allocation size for scsi_host_alloc()

[Martin K. Petersen]

Alexey,

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

> On 2023-01-17 13:23:19 -0800, Bart Van Assche wrote:
>
>  > 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:

> 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!

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH] scsi: hpsa: fix allocation size for scsi_host_alloc()

[Martin K. Petersen]

On Wed, 18 Jan 2023 06:12:55 +0300, Alexey V. Vissarionov wrote:

> On 2023-01-17 13:23:19 -0800, Bart Van Assche wrote:
> 
>  > 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

-- 
Martin K. Petersen	Oracle Linux Engineering