| Message ID | 1457084509-7368-1-git-send-email-mlombard@redhat.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
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?
>-----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 -- 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
>>>>> "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...
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 -- 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
>>>>> "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!
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; }
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(+)