linux-scsi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] be2iscsi: set the boot_kset pointer to NULL in case of failure
@ 2016-03-04  9:41 Maurizio Lombardi
  2016-03-04 11:36 ` Johannes Thumshirn
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Maurizio Lombardi @ 2016-03-04  9:41 UTC (permalink / raw)
  To: jayamohan.kallickal; +Cc: ketan.mukadam, sony.john, linux-scsi

In beiscsi_setup_boot_info(), the boot_kset pointer should be set
to NULL in case of failure otherwise an invalid pointer dereference
may occur later.

Signed-off-by: Maurizio Lombardi <mlombard@redhat.com>
---
 drivers/scsi/be2iscsi/be_main.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/scsi/be2iscsi/be_main.c b/drivers/scsi/be2iscsi/be_main.c
index cb9072a..069e5c5 100644
--- a/drivers/scsi/be2iscsi/be_main.c
+++ b/drivers/scsi/be2iscsi/be_main.c
@@ -4468,6 +4468,7 @@ put_shost:
 	scsi_host_put(phba->shost);
 free_kset:
 	iscsi_boot_destroy_kset(phba->boot_kset);
+	phba->boot_kset = NULL;
 	return -ENOMEM;
 }
 
-- 
Maurizio Lombardi


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] be2iscsi: set the boot_kset pointer to NULL in case of failure
  2016-03-04  9:41 [PATCH] be2iscsi: set the boot_kset pointer to NULL in case of failure Maurizio Lombardi
@ 2016-03-04 11:36 ` Johannes Thumshirn
  2016-03-07  2:26 ` Jitendra Bhivare
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Johannes Thumshirn @ 2016-03-04 11:36 UTC (permalink / raw)
  To: Maurizio Lombardi
  Cc: jayamohan.kallickal, ketan.mukadam, sony.john, linux-scsi

On Fri, Mar 04, 2016 at 10:41:49AM +0100, Maurizio Lombardi wrote:
> In beiscsi_setup_boot_info(), the boot_kset pointer should be set
> to NULL in case of failure otherwise an invalid pointer dereference
> may occur later.
> 
> Signed-off-by: Maurizio Lombardi <mlombard@redhat.com>

Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>

Btw, should this go to stable as well?
-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH] be2iscsi: set the boot_kset pointer to NULL in case of failure
  2016-03-04  9:41 [PATCH] be2iscsi: set the boot_kset pointer to NULL in case of failure Maurizio Lombardi
  2016-03-04 11:36 ` Johannes Thumshirn
@ 2016-03-07  2:26 ` Jitendra Bhivare
  2016-03-08  2:03 ` Martin K. Petersen
  2016-03-09  1:52 ` Martin K. Petersen
  3 siblings, 0 replies; 6+ messages in thread
From: Jitendra Bhivare @ 2016-03-07  2:26 UTC (permalink / raw)
  To: Maurizio Lombardi, jayamohan.kallickal
  Cc: ketan.mukadam, sony.john, linux-scsi

>-----Original Message-----
>From: linux-scsi-owner@vger.kernel.org [mailto:linux-scsi-
>owner@vger.kernel.org] On Behalf Of Maurizio Lombardi
>Sent: Friday, March 04, 2016 3:12 PM
>To: jayamohan.kallickal@avagotech.com
>Cc: ketan.mukadam@avagotech.com; sony.john@avagotech.com; linux-
>scsi@vger.kernel.org
>Subject: [PATCH] be2iscsi: set the boot_kset pointer to NULL in case of
failure
>
>In beiscsi_setup_boot_info(), the boot_kset pointer should be set to NULL
in case
>of failure otherwise an invalid pointer dereference may occur later.
>
>Signed-off-by: Maurizio Lombardi <mlombard@redhat.com>
>---
> drivers/scsi/be2iscsi/be_main.c | 1 +
> 1 file changed, 1 insertion(+)
>
>diff --git a/drivers/scsi/be2iscsi/be_main.c
b/drivers/scsi/be2iscsi/be_main.c
>index cb9072a..069e5c5 100644
>--- a/drivers/scsi/be2iscsi/be_main.c
>+++ b/drivers/scsi/be2iscsi/be_main.c
>@@ -4468,6 +4468,7 @@ put_shost:
> 	scsi_host_put(phba->shost);
> free_kset:
> 	iscsi_boot_destroy_kset(phba->boot_kset);
>+	phba->boot_kset = NULL;
> 	return -ENOMEM;
> }
>
>--
>Maurizio Lombardi
>
>--
>To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of
>a message to majordomo@vger.kernel.org More majordomo info at
>http://vger.kernel.org/majordomo-info.html

Reviewed-by: Jitendra Bhivare <jitendra.bhivare@broadcom.com>

Thanks,

JB

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] be2iscsi: set the boot_kset pointer to NULL in case of failure
  2016-03-04  9:41 [PATCH] be2iscsi: set the boot_kset pointer to NULL in case of failure Maurizio Lombardi
  2016-03-04 11:36 ` Johannes Thumshirn
  2016-03-07  2:26 ` Jitendra Bhivare
@ 2016-03-08  2:03 ` Martin K. Petersen
  2016-03-08 10:28   ` Maurizio Lombardi
  2016-03-09  1:52 ` Martin K. Petersen
  3 siblings, 1 reply; 6+ messages in thread
From: Martin K. Petersen @ 2016-03-08  2:03 UTC (permalink / raw)
  To: Maurizio Lombardi
  Cc: jayamohan.kallickal, ketan.mukadam, sony.john, linux-scsi

>>>>> "Maurizio" == Maurizio Lombardi <mlombard@redhat.com> writes:

Maurizio,

Maurizio> In beiscsi_setup_boot_info(), the boot_kset pointer should be
Maurizio> set to NULL in case of failure otherwise an invalid pointer
Maurizio> dereference may occur later.

iscsi_boot_destroy_kset() checks before deref and the other use location
just checks to see whether it's NULL. Are there places in the core iSCSI
code that use this without checking?

No particular problem with your patch. Just trying to decide whether
it's 4.5 material or not...

-- 
Martin K. Petersen	Oracle Linux Engineering

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] be2iscsi: set the boot_kset pointer to NULL in case of failure
  2016-03-08  2:03 ` Martin K. Petersen
@ 2016-03-08 10:28   ` Maurizio Lombardi
  0 siblings, 0 replies; 6+ messages in thread
From: Maurizio Lombardi @ 2016-03-08 10:28 UTC (permalink / raw)
  To: Martin K. Petersen
  Cc: jayamohan.kallickal, ketan.mukadam, sony.john, linux-scsi



On 03/08/2016 03:03 AM, Martin K. Petersen wrote:
>>>>>> "Maurizio" == Maurizio Lombardi <mlombard@redhat.com> writes:
> 
> Maurizio,
> 
> Maurizio> In beiscsi_setup_boot_info(), the boot_kset pointer should be
> Maurizio> set to NULL in case of failure otherwise an invalid pointer
> Maurizio> dereference may occur later.
> 
> iscsi_boot_destroy_kset() checks before deref and the other use location
> just checks to see whether it's NULL. Are there places in the core iSCSI
> code that use this without checking?

1) At the beginning of the beiscsi_setup_boot_info() function there is
the following check:

----------
/* it has been created previously */
if (phba->boot_kset)
        return 0;
----------

If the function fails and the boot_kset pointer is not set to NULL,
subsequent calls to beiscsi_setup_boot_info() will incorrectly return
success because it assumes that the boot_kset pointer is valid.

2) it is true that iscsi_boot_destroy_kset() checks whether the pointer is NULL
or not, but it the kset has been already destroyed and the pointer is not set to
NULL, then it will dereference an invalid pointer.

Regards,
Maurizio

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] be2iscsi: set the boot_kset pointer to NULL in case of failure
  2016-03-04  9:41 [PATCH] be2iscsi: set the boot_kset pointer to NULL in case of failure Maurizio Lombardi
                   ` (2 preceding siblings ...)
  2016-03-08  2:03 ` Martin K. Petersen
@ 2016-03-09  1:52 ` Martin K. Petersen
  3 siblings, 0 replies; 6+ messages in thread
From: Martin K. Petersen @ 2016-03-09  1:52 UTC (permalink / raw)
  To: Maurizio Lombardi
  Cc: jayamohan.kallickal, ketan.mukadam, sony.john, linux-scsi

>>>>> "Maurizio" == Maurizio Lombardi <mlombard@redhat.com> writes:

Maurizio> In beiscsi_setup_boot_info(), the boot_kset pointer should be
Maurizio> set to NULL in case of failure otherwise an invalid pointer
Maurizio> dereference may occur later.

Applied to 4.5/scsi-fixes.

Thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-03-09  1:52 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-03-04  9:41 [PATCH] be2iscsi: set the boot_kset pointer to NULL in case of failure Maurizio Lombardi
2016-03-04 11:36 ` Johannes Thumshirn
2016-03-07  2:26 ` Jitendra Bhivare
2016-03-08  2:03 ` Martin K. Petersen
2016-03-08 10:28   ` Maurizio Lombardi
2016-03-09  1:52 ` Martin K. Petersen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).