[PATCH] mpt3sas: Ensure the connector_name string is NUL-terminated

[PATCH] mpt3sas: Ensure the connector_name string is NUL-terminated

[Calvin Owens]

We blindly trust the hardware to give us NUL-terminated strings, which
is a bad idea because it doesn't always do that. For example:

  [  481.184784] mpt3sas_cm0: 	enclosure level(0x0000), connector name(     \x3)

In this case, connector_name is four spaces. We got lucky here because
the 2nd byte beyond our character array happens to be a NUL. Fix this
by explicitly writing '\0' to the end of the string to ensure we don't
run off the edge of the world in printk().

Signed-off-by: Calvin Owens <calvinowens@fb.com>
---
 drivers/scsi/mpt3sas/mpt3sas_base.h  |  2 +-
 drivers/scsi/mpt3sas/mpt3sas_scsih.c | 10 ++++++----
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.h b/drivers/scsi/mpt3sas/mpt3sas_base.h
index 892c9be..eb7f5b0 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.h
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.h
@@ -478,7 +478,7 @@ struct _sas_device {
 	u8	pfa_led_on;
 	u8	pend_sas_rphy_add;
 	u8	enclosure_level;
-	u8	connector_name[4];
+	u8	connector_name[5];
 	struct kref refcount;
 };
 
diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
index cd91a68..acabe48 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
@@ -5380,8 +5380,9 @@ _scsih_check_device(struct MPT3SAS_ADAPTER *ioc,
 		     MPI2_SAS_DEVICE0_FLAGS_ENCL_LEVEL_VALID) {
 			sas_device->enclosure_level =
 				le16_to_cpu(sas_device_pg0.EnclosureLevel);
-			memcpy(&sas_device->connector_name[0],
-				&sas_device_pg0.ConnectorName[0], 4);
+			memcpy(sas_device->connector_name,
+				sas_device_pg0.ConnectorName, 4);
+			sas_device->connector_name[4] = '\0';
 		} else {
 			sas_device->enclosure_level = 0;
 			sas_device->connector_name[0] = '\0';
@@ -5508,8 +5509,9 @@ _scsih_add_device(struct MPT3SAS_ADAPTER *ioc, u16 handle, u8 phy_num,
 	if (sas_device_pg0.Flags & MPI2_SAS_DEVICE0_FLAGS_ENCL_LEVEL_VALID) {
 		sas_device->enclosure_level =
 			le16_to_cpu(sas_device_pg0.EnclosureLevel);
-		memcpy(&sas_device->connector_name[0],
-			&sas_device_pg0.ConnectorName[0], 4);
+		memcpy(sas_device->connector_name,
+			sas_device_pg0.ConnectorName, 4);
+		sas_device->connector_name[4] = '\0';
 	} else {
 		sas_device->enclosure_level = 0;
 		sas_device->connector_name[0] = '\0';
-- 
2.8.0.rc2

Re: [PATCH] mpt3sas: Ensure the connector_name string is NUL-terminated

[Martin K. Petersen]

>>>>> "Calvin" == Calvin Owens <calvinowens@fb.com> writes:

Calvin> We blindly trust the hardware to give us NUL-terminated strings,
Calvin> which is a bad idea because it doesn't always do that. For
Calvin> example:

Broadcom folks, please respond to this and other mpt3sas patches in the
queue:

https://patchwork.kernel.org/project/linux-scsi/list/?q=mpt3sas

-- 
Martin K. Petersen	Oracle Linux Engineering

RE: [PATCH] mpt3sas: Ensure the connector_name string is NUL-terminated

[Chaitra Basappa]

Hi,
 Please consider this patch as Acked-by: Chaitra P B
<chaitra.basappa@broadcom.com>


Thanks,
 Chaitra

-----Original Message-----
From: Calvin Owens [mailto:calvinowens@fb.com]
Sent: Thursday, July 28, 2016 10:16 AM
To: Sathya Prakash; Chaitra P B; Suganath Prabu Subramani; James E.J.
Bottomley; Martin K. Petersen
Cc: MPT-FusionLinux.pdl@broadcom.com; linux-scsi@vger.kernel.org;
linux-kernel@vger.kernel.org; kernel-team@fb.com; Calvin Owens
Subject: [PATCH] mpt3sas: Ensure the connector_name string is
NUL-terminated

We blindly trust the hardware to give us NUL-terminated strings, which is
a bad idea because it doesn't always do that. For example:

  [  481.184784] mpt3sas_cm0: 	enclosure level(0x0000), connector name(
\x3)

In this case, connector_name is four spaces. We got lucky here because the
2nd byte beyond our character array happens to be a NUL. Fix this by
explicitly writing '\0' to the end of the string to ensure we don't run
off the edge of the world in printk().

Signed-off-by: Calvin Owens <calvinowens@fb.com>
---
 drivers/scsi/mpt3sas/mpt3sas_base.h  |  2 +-
drivers/scsi/mpt3sas/mpt3sas_scsih.c | 10 ++++++----
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.h
b/drivers/scsi/mpt3sas/mpt3sas_base.h
index 892c9be..eb7f5b0 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.h
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.h
@@ -478,7 +478,7 @@ struct _sas_device {
 	u8	pfa_led_on;
 	u8	pend_sas_rphy_add;
 	u8	enclosure_level;
-	u8	connector_name[4];
+	u8	connector_name[5];
 	struct kref refcount;
 };

diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
index cd91a68..acabe48 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
@@ -5380,8 +5380,9 @@ _scsih_check_device(struct MPT3SAS_ADAPTER *ioc,
 		     MPI2_SAS_DEVICE0_FLAGS_ENCL_LEVEL_VALID) {
 			sas_device->enclosure_level =

le16_to_cpu(sas_device_pg0.EnclosureLevel);
-			memcpy(&sas_device->connector_name[0],
-				&sas_device_pg0.ConnectorName[0], 4);
+			memcpy(sas_device->connector_name,
+				sas_device_pg0.ConnectorName, 4);
+			sas_device->connector_name[4] = '\0';
 		} else {
 			sas_device->enclosure_level = 0;
 			sas_device->connector_name[0] = '\0'; @@ -5508,8
+5509,9 @@ _scsih_add_device(struct MPT3SAS_ADAPTER *ioc, u16 handle, u8
phy_num,
 	if (sas_device_pg0.Flags &
MPI2_SAS_DEVICE0_FLAGS_ENCL_LEVEL_VALID) {
 		sas_device->enclosure_level =
 			le16_to_cpu(sas_device_pg0.EnclosureLevel);
-		memcpy(&sas_device->connector_name[0],
-			&sas_device_pg0.ConnectorName[0], 4);
+		memcpy(sas_device->connector_name,
+			sas_device_pg0.ConnectorName, 4);
+		sas_device->connector_name[4] = '\0';
 	} else {
 		sas_device->enclosure_level = 0;
 		sas_device->connector_name[0] = '\0';
--
2.8.0.rc2

Re: [PATCH] mpt3sas: Ensure the connector_name string is NUL-terminated

[Martin K. Petersen]

>>>>> "Calvin" == Calvin Owens <calvinowens@fb.com> writes:

Calvin> We blindly trust the hardware to give us NUL-terminated strings,
Calvin> which is a bad idea because it doesn't always do that. For
Calvin> example:

Calvin>   [ 481.184784] mpt3sas_cm0: enclosure level(0x0000), connector
Calvin> name( \x3)

Calvin> In this case, connector_name is four spaces. We got lucky here
Calvin> because the 2nd byte beyond our character array happens to be a
Calvin> NUL. Fix this by explicitly writing '\0' to the end of the
Calvin> string to ensure we don't run off the edge of the world in
Calvin> printk().

Applied to 4.9/scsi-queue.

-- 
Martin K. Petersen	Oracle Linux Engineering