[PATCH] libosd: Fix NULL dereference BUG when target is none OSD conformant

[PATCH] libosd: Fix NULL dereference BUG when target is none OSD conformant

[Boaz Harrosh]

Very old OSC's Target had a BUG in the Get/Set attributes where
it was looking in the wrong places for attribute lists length.
If used with the open-osd initiator, the initiator would dereference
a NULL pointer when retrieving system_information attributes.

Checks are added if retrieval of each attributes was successful
before accessing them.

Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
---
 drivers/scsi/osd/osd_initiator.c |   11 +++++++----
 1 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/osd/osd_initiator.c b/drivers/scsi/osd/osd_initiator.c
index eeaec3e..1696130 100644
--- a/drivers/scsi/osd/osd_initiator.c
+++ b/drivers/scsi/osd/osd_initiator.c
@@ -131,7 +131,7 @@ static int _osd_print_system_info(struct osd_dev *od, void *caps)
 
 	pFirst = get_attrs[a++].val_ptr;
 	OSD_INFO("PRODUCT_REVISION_LEVEL [%u]\n",
-		get_unaligned_be32(pFirst));
+		pFirst ? get_unaligned_be32(pFirst) : ~0U);
 
 	pFirst = get_attrs[a++].val_ptr;
 	OSD_INFO("PRODUCT_SERIAL_NUMBER  [%s]\n",
@@ -143,15 +143,18 @@ static int _osd_print_system_info(struct osd_dev *od, void *caps)
 
 	pFirst = get_attrs[a++].val_ptr;
 	OSD_INFO("TOTAL_CAPACITY         [0x%llx]\n",
-		_LLU(get_unaligned_be64(pFirst)));
+		pFirst ? _LLU(get_unaligned_be64(pFirst)) : ~0ULL);
 
 	pFirst = get_attrs[a++].val_ptr;
 	OSD_INFO("USED_CAPACITY          [0x%llx]\n",
-		_LLU(get_unaligned_be64(pFirst)));
+		pFirst ? _LLU(get_unaligned_be64(pFirst)) : ~0ULL);
 
 	pFirst = get_attrs[a++].val_ptr;
 	OSD_INFO("NUMBER_OF_PARTITIONS   [%llu]\n",
-		_LLU(get_unaligned_be64(pFirst)));
+		pFirst ? _LLU(get_unaligned_be64(pFirst)) : ~0ULL);
+
+	if (a >= nelem)
+		goto out;
 
 	/* FIXME: Where are the time utilities */
 	pFirst = get_attrs[a++].val_ptr;
-- 
1.6.0.1

Re: [PATCH] libosd: Fix NULL dereference BUG when target is none OSD conformant

[Boaz Harrosh]

Boaz Harrosh wrote:
> Very old OSC's Target had a BUG in the Get/Set attributes where
> it was looking in the wrong places for attribute lists length.
> If used with the open-osd initiator, the initiator would dereference
> a NULL pointer when retrieving system_information attributes.
> 
> Checks are added if retrieval of each attributes was successful
> before accessing them.
> 
> Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
> ---

Lars I will need your Tested-by: ... here
I did not manage to run with your version of the target. It might crash
farther on now.

(Or if you can identify where on git://git.open-osd.org/osc-osd is the
point that best matches your version and I can test it here)

Thanks in advance
Boaz

Re: [PATCH] libosd: Fix NULL dereference BUG when target is none OSD conformant

[Boaz Harrosh]

Boaz Harrosh wrote:
> Boaz Harrosh wrote:
>> Very old OSC's Target had a BUG in the Get/Set attributes where
>> it was looking in the wrong places for attribute lists length.
>> If used with the open-osd initiator, the initiator would dereference
>> a NULL pointer when retrieving system_information attributes.
>>
>> Checks are added if retrieval of each attributes was successful
>> before accessing them.
>>
>> Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
>> ---
> 
> Lars I will need your Tested-by: ... here
> I did not manage to run with your version of the target. It might crash
> farther on now.
> 

OK That patch will not work for you. I had another one in Q before that
one. I've rebased so it will apply over current scsi-misc.

Sent as reply to first patch.

Sorry for the noise

Boaz

[PATCH version 2] libosd: Fix NULL dereference BUG when target is not OSD conformant

[Boaz Harrosh]

Very old OSC's Target had a BUG in the Get/Set attributes where
it was looking in the wrong places for attribute lists length.
If used with the open-osd initiator, the initiator would dereference
a NULL pointer when retrieving system_information attributes.

Checks are added that retrieval of each attribute is successful
before accessing its value.

Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
---
 drivers/scsi/osd/osd_initiator.c |   11 +++++++----
 1 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/osd/osd_initiator.c b/drivers/scsi/osd/osd_initiator.c
index 0bbbf27..552f58b 100644
--- a/drivers/scsi/osd/osd_initiator.c
+++ b/drivers/scsi/osd/osd_initiator.c
@@ -131,7 +131,7 @@ static int _osd_print_system_info(struct osd_dev *od, void *caps)
 
 	pFirst = get_attrs[a++].val_ptr;
 	OSD_INFO("OSD_ATTR_RI_PRODUCT_REVISION_LEVEL [%u]\n",
-		get_unaligned_be32(pFirst));
+		pFirst ? get_unaligned_be32(pFirst) : ~0U);
 
 	pFirst = get_attrs[a++].val_ptr;
 	OSD_INFO("OSD_ATTR_RI_PRODUCT_SERIAL_NUMBER [%s]\n",
@@ -143,15 +143,18 @@ static int _osd_print_system_info(struct osd_dev *od, void *caps)
 
 	pFirst = get_attrs[a++].val_ptr;
 	OSD_INFO("OSD_ATTR_RI_TOTAL_CAPACITY [0x%llx]\n",
-		_LLU(get_unaligned_be64(pFirst)));
+		pFirst ? _LLU(get_unaligned_be64(pFirst)) : ~0ULL);
 
 	pFirst = get_attrs[a++].val_ptr;
 	OSD_INFO("OSD_ATTR_RI_USED_CAPACITY [0x%llx]\n",
-		_LLU(get_unaligned_be64(pFirst)));
+		pFirst ? _LLU(get_unaligned_be64(pFirst)) : ~0ULL);
 
 	pFirst = get_attrs[a++].val_ptr;
 	OSD_INFO("OSD_ATTR_RI_NUMBER_OF_PARTITIONS [%llu]\n",
-		_LLU(get_unaligned_be64(pFirst)));
+		pFirst ? _LLU(get_unaligned_be64(pFirst)) : ~0ULL);
+
+	if (a >= nelem)
+		goto out;
 
 	/* FIXME: Where are the time utilities */
 	pFirst = get_attrs[a++].val_ptr;
-- 
1.6.0.1

Re: [osd-dev] [PATCH version 2] libosd: Fix NULL dereference BUG when target is not OSD conformant

[Boaz Harrosh]

Boaz Harrosh wrote:
> Very old OSC's Target had a BUG in the Get/Set attributes where
> it was looking in the wrong places for attribute lists length.
> If used with the open-osd initiator, the initiator would dereference
> a NULL pointer when retrieving system_information attributes.
> 
> Checks are added that retrieval of each attribute is successful
> before accessing its value.
> 
> Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
Tested-by: Fuerst Lars <lars.fuerst@thomson.net>

James please submit this patch on top of current scsi-misc for a fix of a serious
BUG. It should be submitted together with the osd patches for 2.6.30.

Thanks in advance
Boaz