All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] [SCSI] ipr: Fix stack overflow in ipr_format_resource_path
@ 2010-06-02 11:16 ` Anton Blanchard
  0 siblings, 0 replies; 10+ messages in thread
From: Anton Blanchard @ 2010-06-02 11:16 UTC (permalink / raw)
  To: brking, wayneb, betabandido; +Cc: linux-scsi, linuxppc-dev


Victor reported an oops during boot with 2.6.34 on a POWER6 JS22:

https://bugzilla.kernel.org/show_bug.cgi?id=16089

Checking ipr microcode levels
Unable to handle kernel paging request for instruction fetch
Faulting instruction address: 0x322d30312d31302c
...
Oops: Kernel access of bad area, sig: 11 [#1]
SMP NR_CPUS=128 NUMA pSeries
last sysfs file:
/sys/devices/pci0000:00/0000:00:01.0/host0/target0:255:255/0:255:255:255/resource_path
Modules linked in:
NIP: 322d30312d31302c LR: 322d30312d31302d CTR: c000000000375bec
REGS: c0000003d360f8f0 TRAP: 0400   Not tainted  (2.6.34-vjj)
MSR: 8000000040009432 <EE,ME,IR,DR>  CR: 28002484  XER: 20000020
TASK = c0000003d587d010[5163] 'iprupdate' THREAD: c0000003d360c000 CPU: 7
GPR00: 322d30312d31302d c0000003d360fb70 c000000000ad07c0 00000000000185a0 
GPR04: 0000000000000001 c0000003d360fb10 04000affffffffff c0000000006a6700 
GPR08: c000000000823383 0000000000000000 0000000000000020 0000000000000000 
GPR12: 000000000000f032 c00000000f622e00 00000000000000ed 0000000000000000 
GPR16: 00000000100b8808 0000000010020000 0000000010020000 0000000010010000 
GPR20: 0000000010010000 0000000000000001 0000000000001000 000000001045eef8 
GPR24: c0000003d360fdf8 302d31342d31302d 43302d30302d3030 2d30332d44352d34 
GPR28: 422d33382d30302d 30302d30302d3030 2d30302d30302d30 302d30302d30302d 
NIP [322d30312d31302c] 0x322d30312d31302c
LR [322d30312d31302d] 0x322d30312d31302d

A stack overflow. It turns out ipr_format_resource_path writes to a
passed in buffer using sprintf which is dangerous and in this case looks
to be the cause of the overflow.

The following patch passes in the length of the buffer and uses snprintf,
which fixes the fail for me. It doesn't fix the other issue, which is
why the loop in ipr_format_resource_path didn't terminate in the first place.
That can be fixed in a follow up patch.

Signed-off-by: Anton Blanchard <anton@samba.org>
---

Index: linux.trees.git/drivers/scsi/ipr.c
===================================================================
--- linux.trees.git.orig/drivers/scsi/ipr.c	2010-05-31 08:51:20.000000000 +1000
+++ linux.trees.git/drivers/scsi/ipr.c	2010-06-02 21:15:41.000000000 +1000
@@ -1132,17 +1132,20 @@ static int ipr_is_same_device(struct ipr
  * ipr_format_resource_path - Format the resource path for printing.
  * @res_path:	resource path
  * @buf:	buffer
+ * @len:	length of the buffer
  *
  * Return value:
  * 	pointer to buffer
  **/
-static char *ipr_format_resource_path(u8 *res_path, char *buffer)
+static char *ipr_format_resource_path(u8 *res_path, char *buffer,
+				      unsigned int len)
 {
 	int i;
+	char *p = buffer;
 
-	sprintf(buffer, "%02X", res_path[0]);
+	p += snprintf(p, buffer + len - p, "%02X", res_path[0]);
 	for (i=1; res_path[i] != 0xff; i++)
-		sprintf(buffer, "%s-%02X", buffer, res_path[i]);
+		p += snprintf(p, buffer + len - p, "-%02X", res_path[i]);
 
 	return buffer;
 }
@@ -1187,7 +1190,8 @@ static void ipr_update_res_entry(struct 
 
 		if (res->sdev && new_path)
 			sdev_printk(KERN_INFO, res->sdev, "Resource path: %s\n",
-				    ipr_format_resource_path(&res->res_path[0], &buffer[0]));
+				    ipr_format_resource_path(&res->res_path[0],
+				    &buffer[0], sizeof(buffer)));
 	} else {
 		res->flags = cfgtew->u.cfgte->flags;
 		if (res->flags & IPR_IS_IOA_RESOURCE)
@@ -1573,7 +1577,8 @@ static void ipr_log_sis64_config_error(s
 		ipr_err_separator;
 
 		ipr_err("Device %d : %s", i + 1,
-			 ipr_format_resource_path(&dev_entry->res_path[0], &buffer[0]));
+			 ipr_format_resource_path(&dev_entry->res_path[0],
+			 &buffer[0], sizeof(buffer)));
 		ipr_log_ext_vpd(&dev_entry->vpd);
 
 		ipr_err("-----New Device Information-----\n");
@@ -1919,13 +1924,15 @@ static void ipr_log64_fabric_path(struct
 
 			ipr_hcam_err(hostrcb, "%s %s: Resource Path=%s\n",
 				     path_active_desc[i].desc, path_state_desc[j].desc,
-				     ipr_format_resource_path(&fabric->res_path[0], &buffer[0]));
+				     ipr_format_resource_path(&fabric->res_path[0],
+				     &buffer[0], sizeof(buffer)));
 			return;
 		}
 	}
 
 	ipr_err("Path state=%02X Resource Path=%s\n", path_state,
-		ipr_format_resource_path(&fabric->res_path[0], &buffer[0]));
+		ipr_format_resource_path(&fabric->res_path[0], &buffer[0],
+		sizeof(buffer)));
 }
 
 static const struct {
@@ -2066,7 +2073,9 @@ static void ipr_log64_path_elem(struct i
 
 			ipr_hcam_err(hostrcb, "%s %s: Resource Path=%s, Link rate=%s, WWN=%08X%08X\n",
 				     path_status_desc[j].desc, path_type_desc[i].desc,
-				     ipr_format_resource_path(&cfg->res_path[0], &buffer[0]),
+				     ipr_format_resource_path(&cfg->res_path[0],
+							      &buffer[0],
+							      sizeof(buffer)),
 				     link_rate[cfg->link_rate & IPR_PHY_LINK_RATE_MASK],
 				     be32_to_cpu(cfg->wwid[0]), be32_to_cpu(cfg->wwid[1]));
 			return;
@@ -2074,7 +2083,8 @@ static void ipr_log64_path_elem(struct i
 	}
 	ipr_hcam_err(hostrcb, "Path element=%02X: Resource Path=%s, Link rate=%s "
 		     "WWN=%08X%08X\n", cfg->type_status,
-		     ipr_format_resource_path(&cfg->res_path[0], &buffer[0]),
+		     ipr_format_resource_path(&cfg->res_path[0], &buffer[0],
+		     sizeof(buffer)),
 		     link_rate[cfg->link_rate & IPR_PHY_LINK_RATE_MASK],
 		     be32_to_cpu(cfg->wwid[0]), be32_to_cpu(cfg->wwid[1]));
 }
@@ -2139,7 +2149,8 @@ static void ipr_log_sis64_array_error(st
 
 	ipr_err("RAID %s Array Configuration: %s\n",
 		error->protection_level,
-		ipr_format_resource_path(&error->last_res_path[0], &buffer[0]));
+		ipr_format_resource_path(&error->last_res_path[0], &buffer[0],
+					 sizeof(buffer)));
 
 	ipr_err_separator;
 
@@ -2160,9 +2171,12 @@ static void ipr_log_sis64_array_error(st
 		ipr_err("Array Member %d:\n", i);
 		ipr_log_ext_vpd(&array_entry->vpd);
 		ipr_err("Current Location: %s",
-			 ipr_format_resource_path(&array_entry->res_path[0], &buffer[0]));
+			 ipr_format_resource_path(&array_entry->res_path[0],
+						  &buffer[0],
+						  sizeof(buffer)));
 		ipr_err("Expected Location: %s",
-			 ipr_format_resource_path(&array_entry->expected_res_path[0], &buffer[0]));
+			 ipr_format_resource_path(&array_entry->expected_res_path[0],
+						  &buffer[0], sizeof(buffer)));
 
 		ipr_err_separator;
 	}
@@ -4099,7 +4113,9 @@ static ssize_t ipr_show_resource_path(st
 	res = (struct ipr_resource_entry *)sdev->hostdata;
 	if (res)
 		len = snprintf(buf, PAGE_SIZE, "%s\n",
-			       ipr_format_resource_path(&res->res_path[0], &buffer[0]));
+			       ipr_format_resource_path(&res->res_path[0],
+							&buffer[0],
+							sizeof(buffer)));
 	spin_unlock_irqrestore(ioa_cfg->host->host_lock, lock_flags);
 	return len;
 }
@@ -4351,7 +4367,9 @@ static int ipr_slave_configure(struct sc
 			scsi_adjust_queue_depth(sdev, 0, sdev->host->cmd_per_lun);
 		if (ioa_cfg->sis64)
 			sdev_printk(KERN_INFO, sdev, "Resource path: %s\n",
-			            ipr_format_resource_path(&res->res_path[0], &buffer[0]));
+			            ipr_format_resource_path(&res->res_path[0],
+							     &buffer[0],
+							     sizeof(buffer)));
 		return 0;
 	}
 	spin_unlock_irqrestore(ioa_cfg->host->host_lock, lock_flags);
Index: linux.trees.git/drivers/scsi/ipr.h
===================================================================
--- linux.trees.git.orig/drivers/scsi/ipr.h	2010-05-31 08:51:20.000000000 +1000
+++ linux.trees.git/drivers/scsi/ipr.h	2010-06-02 21:15:41.000000000 +1000
@@ -1685,7 +1685,8 @@ struct ipr_ucode_image_header {
 		if ((hostrcb)->ioa_cfg->sis64) {			\
 			printk(KERN_ERR IPR_NAME ": %s: " fmt, 		\
 				ipr_format_resource_path(&hostrcb->hcam.u.error64.fd_res_path[0], \
-					&hostrcb->rp_buffer[0]),	\
+					&hostrcb->rp_buffer[0],		\
+					sizeof(hostrcb->rp_buffer)),	\
 				__VA_ARGS__);				\
 		} else {						\
 			ipr_ra_err((hostrcb)->ioa_cfg,			\

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

* [PATCH] [SCSI] ipr: Fix stack overflow in ipr_format_resource_path
@ 2010-06-02 11:16 ` Anton Blanchard
  0 siblings, 0 replies; 10+ messages in thread
From: Anton Blanchard @ 2010-06-02 11:16 UTC (permalink / raw)
  To: brking, wayneb, betabandido; +Cc: linuxppc-dev, linux-scsi


Victor reported an oops during boot with 2.6.34 on a POWER6 JS22:

https://bugzilla.kernel.org/show_bug.cgi?id=16089

Checking ipr microcode levels
Unable to handle kernel paging request for instruction fetch
Faulting instruction address: 0x322d30312d31302c
...
Oops: Kernel access of bad area, sig: 11 [#1]
SMP NR_CPUS=128 NUMA pSeries
last sysfs file:
/sys/devices/pci0000:00/0000:00:01.0/host0/target0:255:255/0:255:255:255/resource_path
Modules linked in:
NIP: 322d30312d31302c LR: 322d30312d31302d CTR: c000000000375bec
REGS: c0000003d360f8f0 TRAP: 0400   Not tainted  (2.6.34-vjj)
MSR: 8000000040009432 <EE,ME,IR,DR>  CR: 28002484  XER: 20000020
TASK = c0000003d587d010[5163] 'iprupdate' THREAD: c0000003d360c000 CPU: 7
GPR00: 322d30312d31302d c0000003d360fb70 c000000000ad07c0 00000000000185a0 
GPR04: 0000000000000001 c0000003d360fb10 04000affffffffff c0000000006a6700 
GPR08: c000000000823383 0000000000000000 0000000000000020 0000000000000000 
GPR12: 000000000000f032 c00000000f622e00 00000000000000ed 0000000000000000 
GPR16: 00000000100b8808 0000000010020000 0000000010020000 0000000010010000 
GPR20: 0000000010010000 0000000000000001 0000000000001000 000000001045eef8 
GPR24: c0000003d360fdf8 302d31342d31302d 43302d30302d3030 2d30332d44352d34 
GPR28: 422d33382d30302d 30302d30302d3030 2d30302d30302d30 302d30302d30302d 
NIP [322d30312d31302c] 0x322d30312d31302c
LR [322d30312d31302d] 0x322d30312d31302d

A stack overflow. It turns out ipr_format_resource_path writes to a
passed in buffer using sprintf which is dangerous and in this case looks
to be the cause of the overflow.

The following patch passes in the length of the buffer and uses snprintf,
which fixes the fail for me. It doesn't fix the other issue, which is
why the loop in ipr_format_resource_path didn't terminate in the first place.
That can be fixed in a follow up patch.

Signed-off-by: Anton Blanchard <anton@samba.org>
---

Index: linux.trees.git/drivers/scsi/ipr.c
===================================================================
--- linux.trees.git.orig/drivers/scsi/ipr.c	2010-05-31 08:51:20.000000000 +1000
+++ linux.trees.git/drivers/scsi/ipr.c	2010-06-02 21:15:41.000000000 +1000
@@ -1132,17 +1132,20 @@ static int ipr_is_same_device(struct ipr
  * ipr_format_resource_path - Format the resource path for printing.
  * @res_path:	resource path
  * @buf:	buffer
+ * @len:	length of the buffer
  *
  * Return value:
  * 	pointer to buffer
  **/
-static char *ipr_format_resource_path(u8 *res_path, char *buffer)
+static char *ipr_format_resource_path(u8 *res_path, char *buffer,
+				      unsigned int len)
 {
 	int i;
+	char *p = buffer;
 
-	sprintf(buffer, "%02X", res_path[0]);
+	p += snprintf(p, buffer + len - p, "%02X", res_path[0]);
 	for (i=1; res_path[i] != 0xff; i++)
-		sprintf(buffer, "%s-%02X", buffer, res_path[i]);
+		p += snprintf(p, buffer + len - p, "-%02X", res_path[i]);
 
 	return buffer;
 }
@@ -1187,7 +1190,8 @@ static void ipr_update_res_entry(struct 
 
 		if (res->sdev && new_path)
 			sdev_printk(KERN_INFO, res->sdev, "Resource path: %s\n",
-				    ipr_format_resource_path(&res->res_path[0], &buffer[0]));
+				    ipr_format_resource_path(&res->res_path[0],
+				    &buffer[0], sizeof(buffer)));
 	} else {
 		res->flags = cfgtew->u.cfgte->flags;
 		if (res->flags & IPR_IS_IOA_RESOURCE)
@@ -1573,7 +1577,8 @@ static void ipr_log_sis64_config_error(s
 		ipr_err_separator;
 
 		ipr_err("Device %d : %s", i + 1,
-			 ipr_format_resource_path(&dev_entry->res_path[0], &buffer[0]));
+			 ipr_format_resource_path(&dev_entry->res_path[0],
+			 &buffer[0], sizeof(buffer)));
 		ipr_log_ext_vpd(&dev_entry->vpd);
 
 		ipr_err("-----New Device Information-----\n");
@@ -1919,13 +1924,15 @@ static void ipr_log64_fabric_path(struct
 
 			ipr_hcam_err(hostrcb, "%s %s: Resource Path=%s\n",
 				     path_active_desc[i].desc, path_state_desc[j].desc,
-				     ipr_format_resource_path(&fabric->res_path[0], &buffer[0]));
+				     ipr_format_resource_path(&fabric->res_path[0],
+				     &buffer[0], sizeof(buffer)));
 			return;
 		}
 	}
 
 	ipr_err("Path state=%02X Resource Path=%s\n", path_state,
-		ipr_format_resource_path(&fabric->res_path[0], &buffer[0]));
+		ipr_format_resource_path(&fabric->res_path[0], &buffer[0],
+		sizeof(buffer)));
 }
 
 static const struct {
@@ -2066,7 +2073,9 @@ static void ipr_log64_path_elem(struct i
 
 			ipr_hcam_err(hostrcb, "%s %s: Resource Path=%s, Link rate=%s, WWN=%08X%08X\n",
 				     path_status_desc[j].desc, path_type_desc[i].desc,
-				     ipr_format_resource_path(&cfg->res_path[0], &buffer[0]),
+				     ipr_format_resource_path(&cfg->res_path[0],
+							      &buffer[0],
+							      sizeof(buffer)),
 				     link_rate[cfg->link_rate & IPR_PHY_LINK_RATE_MASK],
 				     be32_to_cpu(cfg->wwid[0]), be32_to_cpu(cfg->wwid[1]));
 			return;
@@ -2074,7 +2083,8 @@ static void ipr_log64_path_elem(struct i
 	}
 	ipr_hcam_err(hostrcb, "Path element=%02X: Resource Path=%s, Link rate=%s "
 		     "WWN=%08X%08X\n", cfg->type_status,
-		     ipr_format_resource_path(&cfg->res_path[0], &buffer[0]),
+		     ipr_format_resource_path(&cfg->res_path[0], &buffer[0],
+		     sizeof(buffer)),
 		     link_rate[cfg->link_rate & IPR_PHY_LINK_RATE_MASK],
 		     be32_to_cpu(cfg->wwid[0]), be32_to_cpu(cfg->wwid[1]));
 }
@@ -2139,7 +2149,8 @@ static void ipr_log_sis64_array_error(st
 
 	ipr_err("RAID %s Array Configuration: %s\n",
 		error->protection_level,
-		ipr_format_resource_path(&error->last_res_path[0], &buffer[0]));
+		ipr_format_resource_path(&error->last_res_path[0], &buffer[0],
+					 sizeof(buffer)));
 
 	ipr_err_separator;
 
@@ -2160,9 +2171,12 @@ static void ipr_log_sis64_array_error(st
 		ipr_err("Array Member %d:\n", i);
 		ipr_log_ext_vpd(&array_entry->vpd);
 		ipr_err("Current Location: %s",
-			 ipr_format_resource_path(&array_entry->res_path[0], &buffer[0]));
+			 ipr_format_resource_path(&array_entry->res_path[0],
+						  &buffer[0],
+						  sizeof(buffer)));
 		ipr_err("Expected Location: %s",
-			 ipr_format_resource_path(&array_entry->expected_res_path[0], &buffer[0]));
+			 ipr_format_resource_path(&array_entry->expected_res_path[0],
+						  &buffer[0], sizeof(buffer)));
 
 		ipr_err_separator;
 	}
@@ -4099,7 +4113,9 @@ static ssize_t ipr_show_resource_path(st
 	res = (struct ipr_resource_entry *)sdev->hostdata;
 	if (res)
 		len = snprintf(buf, PAGE_SIZE, "%s\n",
-			       ipr_format_resource_path(&res->res_path[0], &buffer[0]));
+			       ipr_format_resource_path(&res->res_path[0],
+							&buffer[0],
+							sizeof(buffer)));
 	spin_unlock_irqrestore(ioa_cfg->host->host_lock, lock_flags);
 	return len;
 }
@@ -4351,7 +4367,9 @@ static int ipr_slave_configure(struct sc
 			scsi_adjust_queue_depth(sdev, 0, sdev->host->cmd_per_lun);
 		if (ioa_cfg->sis64)
 			sdev_printk(KERN_INFO, sdev, "Resource path: %s\n",
-			            ipr_format_resource_path(&res->res_path[0], &buffer[0]));
+			            ipr_format_resource_path(&res->res_path[0],
+							     &buffer[0],
+							     sizeof(buffer)));
 		return 0;
 	}
 	spin_unlock_irqrestore(ioa_cfg->host->host_lock, lock_flags);
Index: linux.trees.git/drivers/scsi/ipr.h
===================================================================
--- linux.trees.git.orig/drivers/scsi/ipr.h	2010-05-31 08:51:20.000000000 +1000
+++ linux.trees.git/drivers/scsi/ipr.h	2010-06-02 21:15:41.000000000 +1000
@@ -1685,7 +1685,8 @@ struct ipr_ucode_image_header {
 		if ((hostrcb)->ioa_cfg->sis64) {			\
 			printk(KERN_ERR IPR_NAME ": %s: " fmt, 		\
 				ipr_format_resource_path(&hostrcb->hcam.u.error64.fd_res_path[0], \
-					&hostrcb->rp_buffer[0]),	\
+					&hostrcb->rp_buffer[0],		\
+					sizeof(hostrcb->rp_buffer)),	\
 				__VA_ARGS__);				\
 		} else {						\
 			ipr_ra_err((hostrcb)->ioa_cfg,			\

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

* Re: [PATCH] [SCSI] ipr: Fix stack overflow in ipr_format_resource_path
  2010-06-02 11:16 ` Anton Blanchard
@ 2010-06-03  4:14   ` Wayne Boyer
  -1 siblings, 0 replies; 10+ messages in thread
From: Wayne Boyer @ 2010-06-03  4:14 UTC (permalink / raw)
  To: Anton Blanchard; +Cc: brking, betabandido, linux-scsi, linuxppc-dev

Hi Anton,

We also saw a variation of this problem last week and I have an alternative
patch that I'd prefer over this one.  I'll post the patch in a separate
email.

Thanks,

-- 
Wayne Boyer
IBM - Beaverton, Oregon
LTC S/W Development - eServerIO
(503) 578-5236, T/L 775-5236


On 06/02/2010 04:16 AM, Anton Blanchard wrote:
> 
> Victor reported an oops during boot with 2.6.34 on a POWER6 JS22:
> 
> https://bugzilla.kernel.org/show_bug.cgi?id=16089
> 
> Checking ipr microcode levels
> Unable to handle kernel paging request for instruction fetch
> Faulting instruction address: 0x322d30312d31302c
> ...
> Oops: Kernel access of bad area, sig: 11 [#1]
> SMP NR_CPUS=128 NUMA pSeries
> last sysfs file:
> /sys/devices/pci0000:00/0000:00:01.0/host0/target0:255:255/0:255:255:255/resource_path
> Modules linked in:
> NIP: 322d30312d31302c LR: 322d30312d31302d CTR: c000000000375bec
> REGS: c0000003d360f8f0 TRAP: 0400   Not tainted  (2.6.34-vjj)
> MSR: 8000000040009432 <EE,ME,IR,DR>  CR: 28002484  XER: 20000020
> TASK = c0000003d587d010[5163] 'iprupdate' THREAD: c0000003d360c000 CPU: 7
> GPR00: 322d30312d31302d c0000003d360fb70 c000000000ad07c0 00000000000185a0 
> GPR04: 0000000000000001 c0000003d360fb10 04000affffffffff c0000000006a6700 
> GPR08: c000000000823383 0000000000000000 0000000000000020 0000000000000000 
> GPR12: 000000000000f032 c00000000f622e00 00000000000000ed 0000000000000000 
> GPR16: 00000000100b8808 0000000010020000 0000000010020000 0000000010010000 
> GPR20: 0000000010010000 0000000000000001 0000000000001000 000000001045eef8 
> GPR24: c0000003d360fdf8 302d31342d31302d 43302d30302d3030 2d30332d44352d34 
> GPR28: 422d33382d30302d 30302d30302d3030 2d30302d30302d30 302d30302d30302d 
> NIP [322d30312d31302c] 0x322d30312d31302c
> LR [322d30312d31302d] 0x322d30312d31302d
> 
> A stack overflow. It turns out ipr_format_resource_path writes to a
> passed in buffer using sprintf which is dangerous and in this case looks
> to be the cause of the overflow.
> 
> The following patch passes in the length of the buffer and uses snprintf,
> which fixes the fail for me. It doesn't fix the other issue, which is
> why the loop in ipr_format_resource_path didn't terminate in the first place.
> That can be fixed in a follow up patch.
> 
> Signed-off-by: Anton Blanchard <anton@samba.org>
> ---
> 
> Index: linux.trees.git/drivers/scsi/ipr.c
> ===================================================================
> --- linux.trees.git.orig/drivers/scsi/ipr.c	2010-05-31 08:51:20.000000000 +1000
> +++ linux.trees.git/drivers/scsi/ipr.c	2010-06-02 21:15:41.000000000 +1000
> @@ -1132,17 +1132,20 @@ static int ipr_is_same_device(struct ipr
>   * ipr_format_resource_path - Format the resource path for printing.
>   * @res_path:	resource path
>   * @buf:	buffer
> + * @len:	length of the buffer
>   *
>   * Return value:
>   * 	pointer to buffer
>   **/
> -static char *ipr_format_resource_path(u8 *res_path, char *buffer)
> +static char *ipr_format_resource_path(u8 *res_path, char *buffer,
> +				      unsigned int len)
>  {
>  	int i;
> +	char *p = buffer;
> 
> -	sprintf(buffer, "%02X", res_path[0]);
> +	p += snprintf(p, buffer + len - p, "%02X", res_path[0]);
>  	for (i=1; res_path[i] != 0xff; i++)
> -		sprintf(buffer, "%s-%02X", buffer, res_path[i]);
> +		p += snprintf(p, buffer + len - p, "-%02X", res_path[i]);
> 
>  	return buffer;
>  }
> @@ -1187,7 +1190,8 @@ static void ipr_update_res_entry(struct 
> 
>  		if (res->sdev && new_path)
>  			sdev_printk(KERN_INFO, res->sdev, "Resource path: %s\n",
> -				    ipr_format_resource_path(&res->res_path[0], &buffer[0]));
> +				    ipr_format_resource_path(&res->res_path[0],
> +				    &buffer[0], sizeof(buffer)));
>  	} else {
>  		res->flags = cfgtew->u.cfgte->flags;
>  		if (res->flags & IPR_IS_IOA_RESOURCE)
> @@ -1573,7 +1577,8 @@ static void ipr_log_sis64_config_error(s
>  		ipr_err_separator;
> 
>  		ipr_err("Device %d : %s", i + 1,
> -			 ipr_format_resource_path(&dev_entry->res_path[0], &buffer[0]));
> +			 ipr_format_resource_path(&dev_entry->res_path[0],
> +			 &buffer[0], sizeof(buffer)));
>  		ipr_log_ext_vpd(&dev_entry->vpd);
> 
>  		ipr_err("-----New Device Information-----\n");
> @@ -1919,13 +1924,15 @@ static void ipr_log64_fabric_path(struct
> 
>  			ipr_hcam_err(hostrcb, "%s %s: Resource Path=%s\n",
>  				     path_active_desc[i].desc, path_state_desc[j].desc,
> -				     ipr_format_resource_path(&fabric->res_path[0], &buffer[0]));
> +				     ipr_format_resource_path(&fabric->res_path[0],
> +				     &buffer[0], sizeof(buffer)));
>  			return;
>  		}
>  	}
> 
>  	ipr_err("Path state=%02X Resource Path=%s\n", path_state,
> -		ipr_format_resource_path(&fabric->res_path[0], &buffer[0]));
> +		ipr_format_resource_path(&fabric->res_path[0], &buffer[0],
> +		sizeof(buffer)));
>  }
> 
>  static const struct {
> @@ -2066,7 +2073,9 @@ static void ipr_log64_path_elem(struct i
> 
>  			ipr_hcam_err(hostrcb, "%s %s: Resource Path=%s, Link rate=%s, WWN=%08X%08X\n",
>  				     path_status_desc[j].desc, path_type_desc[i].desc,
> -				     ipr_format_resource_path(&cfg->res_path[0], &buffer[0]),
> +				     ipr_format_resource_path(&cfg->res_path[0],
> +							      &buffer[0],
> +							      sizeof(buffer)),
>  				     link_rate[cfg->link_rate & IPR_PHY_LINK_RATE_MASK],
>  				     be32_to_cpu(cfg->wwid[0]), be32_to_cpu(cfg->wwid[1]));
>  			return;
> @@ -2074,7 +2083,8 @@ static void ipr_log64_path_elem(struct i
>  	}
>  	ipr_hcam_err(hostrcb, "Path element=%02X: Resource Path=%s, Link rate=%s "
>  		     "WWN=%08X%08X\n", cfg->type_status,
> -		     ipr_format_resource_path(&cfg->res_path[0], &buffer[0]),
> +		     ipr_format_resource_path(&cfg->res_path[0], &buffer[0],
> +		     sizeof(buffer)),
>  		     link_rate[cfg->link_rate & IPR_PHY_LINK_RATE_MASK],
>  		     be32_to_cpu(cfg->wwid[0]), be32_to_cpu(cfg->wwid[1]));
>  }
> @@ -2139,7 +2149,8 @@ static void ipr_log_sis64_array_error(st
> 
>  	ipr_err("RAID %s Array Configuration: %s\n",
>  		error->protection_level,
> -		ipr_format_resource_path(&error->last_res_path[0], &buffer[0]));
> +		ipr_format_resource_path(&error->last_res_path[0], &buffer[0],
> +					 sizeof(buffer)));
> 
>  	ipr_err_separator;
> 
> @@ -2160,9 +2171,12 @@ static void ipr_log_sis64_array_error(st
>  		ipr_err("Array Member %d:\n", i);
>  		ipr_log_ext_vpd(&array_entry->vpd);
>  		ipr_err("Current Location: %s",
> -			 ipr_format_resource_path(&array_entry->res_path[0], &buffer[0]));
> +			 ipr_format_resource_path(&array_entry->res_path[0],
> +						  &buffer[0],
> +						  sizeof(buffer)));
>  		ipr_err("Expected Location: %s",
> -			 ipr_format_resource_path(&array_entry->expected_res_path[0], &buffer[0]));
> +			 ipr_format_resource_path(&array_entry->expected_res_path[0],
> +						  &buffer[0], sizeof(buffer)));
> 
>  		ipr_err_separator;
>  	}
> @@ -4099,7 +4113,9 @@ static ssize_t ipr_show_resource_path(st
>  	res = (struct ipr_resource_entry *)sdev->hostdata;
>  	if (res)
>  		len = snprintf(buf, PAGE_SIZE, "%s\n",
> -			       ipr_format_resource_path(&res->res_path[0], &buffer[0]));
> +			       ipr_format_resource_path(&res->res_path[0],
> +							&buffer[0],
> +							sizeof(buffer)));
>  	spin_unlock_irqrestore(ioa_cfg->host->host_lock, lock_flags);
>  	return len;
>  }
> @@ -4351,7 +4367,9 @@ static int ipr_slave_configure(struct sc
>  			scsi_adjust_queue_depth(sdev, 0, sdev->host->cmd_per_lun);
>  		if (ioa_cfg->sis64)
>  			sdev_printk(KERN_INFO, sdev, "Resource path: %s\n",
> -			            ipr_format_resource_path(&res->res_path[0], &buffer[0]));
> +			            ipr_format_resource_path(&res->res_path[0],
> +							     &buffer[0],
> +							     sizeof(buffer)));
>  		return 0;
>  	}
>  	spin_unlock_irqrestore(ioa_cfg->host->host_lock, lock_flags);
> Index: linux.trees.git/drivers/scsi/ipr.h
> ===================================================================
> --- linux.trees.git.orig/drivers/scsi/ipr.h	2010-05-31 08:51:20.000000000 +1000
> +++ linux.trees.git/drivers/scsi/ipr.h	2010-06-02 21:15:41.000000000 +1000
> @@ -1685,7 +1685,8 @@ struct ipr_ucode_image_header {
>  		if ((hostrcb)->ioa_cfg->sis64) {			\
>  			printk(KERN_ERR IPR_NAME ": %s: " fmt, 		\
>  				ipr_format_resource_path(&hostrcb->hcam.u.error64.fd_res_path[0], \
> -					&hostrcb->rp_buffer[0]),	\
> +					&hostrcb->rp_buffer[0],		\
> +					sizeof(hostrcb->rp_buffer)),	\
>  				__VA_ARGS__);				\
>  		} else {						\
>  			ipr_ra_err((hostrcb)->ioa_cfg,			\
> --
> 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] 10+ messages in thread

* Re: [PATCH] [SCSI] ipr: Fix stack overflow in ipr_format_resource_path
@ 2010-06-03  4:14   ` Wayne Boyer
  0 siblings, 0 replies; 10+ messages in thread
From: Wayne Boyer @ 2010-06-03  4:14 UTC (permalink / raw)
  To: Anton Blanchard; +Cc: betabandido, linux-scsi, linuxppc-dev

Hi Anton,

We also saw a variation of this problem last week and I have an alternative
patch that I'd prefer over this one.  I'll post the patch in a separate
email.

Thanks,

-- 
Wayne Boyer
IBM - Beaverton, Oregon
LTC S/W Development - eServerIO
(503) 578-5236, T/L 775-5236


On 06/02/2010 04:16 AM, Anton Blanchard wrote:
> 
> Victor reported an oops during boot with 2.6.34 on a POWER6 JS22:
> 
> https://bugzilla.kernel.org/show_bug.cgi?id=16089
> 
> Checking ipr microcode levels
> Unable to handle kernel paging request for instruction fetch
> Faulting instruction address: 0x322d30312d31302c
> ...
> Oops: Kernel access of bad area, sig: 11 [#1]
> SMP NR_CPUS=128 NUMA pSeries
> last sysfs file:
> /sys/devices/pci0000:00/0000:00:01.0/host0/target0:255:255/0:255:255:255/resource_path
> Modules linked in:
> NIP: 322d30312d31302c LR: 322d30312d31302d CTR: c000000000375bec
> REGS: c0000003d360f8f0 TRAP: 0400   Not tainted  (2.6.34-vjj)
> MSR: 8000000040009432 <EE,ME,IR,DR>  CR: 28002484  XER: 20000020
> TASK = c0000003d587d010[5163] 'iprupdate' THREAD: c0000003d360c000 CPU: 7
> GPR00: 322d30312d31302d c0000003d360fb70 c000000000ad07c0 00000000000185a0 
> GPR04: 0000000000000001 c0000003d360fb10 04000affffffffff c0000000006a6700 
> GPR08: c000000000823383 0000000000000000 0000000000000020 0000000000000000 
> GPR12: 000000000000f032 c00000000f622e00 00000000000000ed 0000000000000000 
> GPR16: 00000000100b8808 0000000010020000 0000000010020000 0000000010010000 
> GPR20: 0000000010010000 0000000000000001 0000000000001000 000000001045eef8 
> GPR24: c0000003d360fdf8 302d31342d31302d 43302d30302d3030 2d30332d44352d34 
> GPR28: 422d33382d30302d 30302d30302d3030 2d30302d30302d30 302d30302d30302d 
> NIP [322d30312d31302c] 0x322d30312d31302c
> LR [322d30312d31302d] 0x322d30312d31302d
> 
> A stack overflow. It turns out ipr_format_resource_path writes to a
> passed in buffer using sprintf which is dangerous and in this case looks
> to be the cause of the overflow.
> 
> The following patch passes in the length of the buffer and uses snprintf,
> which fixes the fail for me. It doesn't fix the other issue, which is
> why the loop in ipr_format_resource_path didn't terminate in the first place.
> That can be fixed in a follow up patch.
> 
> Signed-off-by: Anton Blanchard <anton@samba.org>
> ---
> 
> Index: linux.trees.git/drivers/scsi/ipr.c
> ===================================================================
> --- linux.trees.git.orig/drivers/scsi/ipr.c	2010-05-31 08:51:20.000000000 +1000
> +++ linux.trees.git/drivers/scsi/ipr.c	2010-06-02 21:15:41.000000000 +1000
> @@ -1132,17 +1132,20 @@ static int ipr_is_same_device(struct ipr
>   * ipr_format_resource_path - Format the resource path for printing.
>   * @res_path:	resource path
>   * @buf:	buffer
> + * @len:	length of the buffer
>   *
>   * Return value:
>   * 	pointer to buffer
>   **/
> -static char *ipr_format_resource_path(u8 *res_path, char *buffer)
> +static char *ipr_format_resource_path(u8 *res_path, char *buffer,
> +				      unsigned int len)
>  {
>  	int i;
> +	char *p = buffer;
> 
> -	sprintf(buffer, "%02X", res_path[0]);
> +	p += snprintf(p, buffer + len - p, "%02X", res_path[0]);
>  	for (i=1; res_path[i] != 0xff; i++)
> -		sprintf(buffer, "%s-%02X", buffer, res_path[i]);
> +		p += snprintf(p, buffer + len - p, "-%02X", res_path[i]);
> 
>  	return buffer;
>  }
> @@ -1187,7 +1190,8 @@ static void ipr_update_res_entry(struct 
> 
>  		if (res->sdev && new_path)
>  			sdev_printk(KERN_INFO, res->sdev, "Resource path: %s\n",
> -				    ipr_format_resource_path(&res->res_path[0], &buffer[0]));
> +				    ipr_format_resource_path(&res->res_path[0],
> +				    &buffer[0], sizeof(buffer)));
>  	} else {
>  		res->flags = cfgtew->u.cfgte->flags;
>  		if (res->flags & IPR_IS_IOA_RESOURCE)
> @@ -1573,7 +1577,8 @@ static void ipr_log_sis64_config_error(s
>  		ipr_err_separator;
> 
>  		ipr_err("Device %d : %s", i + 1,
> -			 ipr_format_resource_path(&dev_entry->res_path[0], &buffer[0]));
> +			 ipr_format_resource_path(&dev_entry->res_path[0],
> +			 &buffer[0], sizeof(buffer)));
>  		ipr_log_ext_vpd(&dev_entry->vpd);
> 
>  		ipr_err("-----New Device Information-----\n");
> @@ -1919,13 +1924,15 @@ static void ipr_log64_fabric_path(struct
> 
>  			ipr_hcam_err(hostrcb, "%s %s: Resource Path=%s\n",
>  				     path_active_desc[i].desc, path_state_desc[j].desc,
> -				     ipr_format_resource_path(&fabric->res_path[0], &buffer[0]));
> +				     ipr_format_resource_path(&fabric->res_path[0],
> +				     &buffer[0], sizeof(buffer)));
>  			return;
>  		}
>  	}
> 
>  	ipr_err("Path state=%02X Resource Path=%s\n", path_state,
> -		ipr_format_resource_path(&fabric->res_path[0], &buffer[0]));
> +		ipr_format_resource_path(&fabric->res_path[0], &buffer[0],
> +		sizeof(buffer)));
>  }
> 
>  static const struct {
> @@ -2066,7 +2073,9 @@ static void ipr_log64_path_elem(struct i
> 
>  			ipr_hcam_err(hostrcb, "%s %s: Resource Path=%s, Link rate=%s, WWN=%08X%08X\n",
>  				     path_status_desc[j].desc, path_type_desc[i].desc,
> -				     ipr_format_resource_path(&cfg->res_path[0], &buffer[0]),
> +				     ipr_format_resource_path(&cfg->res_path[0],
> +							      &buffer[0],
> +							      sizeof(buffer)),
>  				     link_rate[cfg->link_rate & IPR_PHY_LINK_RATE_MASK],
>  				     be32_to_cpu(cfg->wwid[0]), be32_to_cpu(cfg->wwid[1]));
>  			return;
> @@ -2074,7 +2083,8 @@ static void ipr_log64_path_elem(struct i
>  	}
>  	ipr_hcam_err(hostrcb, "Path element=%02X: Resource Path=%s, Link rate=%s "
>  		     "WWN=%08X%08X\n", cfg->type_status,
> -		     ipr_format_resource_path(&cfg->res_path[0], &buffer[0]),
> +		     ipr_format_resource_path(&cfg->res_path[0], &buffer[0],
> +		     sizeof(buffer)),
>  		     link_rate[cfg->link_rate & IPR_PHY_LINK_RATE_MASK],
>  		     be32_to_cpu(cfg->wwid[0]), be32_to_cpu(cfg->wwid[1]));
>  }
> @@ -2139,7 +2149,8 @@ static void ipr_log_sis64_array_error(st
> 
>  	ipr_err("RAID %s Array Configuration: %s\n",
>  		error->protection_level,
> -		ipr_format_resource_path(&error->last_res_path[0], &buffer[0]));
> +		ipr_format_resource_path(&error->last_res_path[0], &buffer[0],
> +					 sizeof(buffer)));
> 
>  	ipr_err_separator;
> 
> @@ -2160,9 +2171,12 @@ static void ipr_log_sis64_array_error(st
>  		ipr_err("Array Member %d:\n", i);
>  		ipr_log_ext_vpd(&array_entry->vpd);
>  		ipr_err("Current Location: %s",
> -			 ipr_format_resource_path(&array_entry->res_path[0], &buffer[0]));
> +			 ipr_format_resource_path(&array_entry->res_path[0],
> +						  &buffer[0],
> +						  sizeof(buffer)));
>  		ipr_err("Expected Location: %s",
> -			 ipr_format_resource_path(&array_entry->expected_res_path[0], &buffer[0]));
> +			 ipr_format_resource_path(&array_entry->expected_res_path[0],
> +						  &buffer[0], sizeof(buffer)));
> 
>  		ipr_err_separator;
>  	}
> @@ -4099,7 +4113,9 @@ static ssize_t ipr_show_resource_path(st
>  	res = (struct ipr_resource_entry *)sdev->hostdata;
>  	if (res)
>  		len = snprintf(buf, PAGE_SIZE, "%s\n",
> -			       ipr_format_resource_path(&res->res_path[0], &buffer[0]));
> +			       ipr_format_resource_path(&res->res_path[0],
> +							&buffer[0],
> +							sizeof(buffer)));
>  	spin_unlock_irqrestore(ioa_cfg->host->host_lock, lock_flags);
>  	return len;
>  }
> @@ -4351,7 +4367,9 @@ static int ipr_slave_configure(struct sc
>  			scsi_adjust_queue_depth(sdev, 0, sdev->host->cmd_per_lun);
>  		if (ioa_cfg->sis64)
>  			sdev_printk(KERN_INFO, sdev, "Resource path: %s\n",
> -			            ipr_format_resource_path(&res->res_path[0], &buffer[0]));
> +			            ipr_format_resource_path(&res->res_path[0],
> +							     &buffer[0],
> +							     sizeof(buffer)));
>  		return 0;
>  	}
>  	spin_unlock_irqrestore(ioa_cfg->host->host_lock, lock_flags);
> Index: linux.trees.git/drivers/scsi/ipr.h
> ===================================================================
> --- linux.trees.git.orig/drivers/scsi/ipr.h	2010-05-31 08:51:20.000000000 +1000
> +++ linux.trees.git/drivers/scsi/ipr.h	2010-06-02 21:15:41.000000000 +1000
> @@ -1685,7 +1685,8 @@ struct ipr_ucode_image_header {
>  		if ((hostrcb)->ioa_cfg->sis64) {			\
>  			printk(KERN_ERR IPR_NAME ": %s: " fmt, 		\
>  				ipr_format_resource_path(&hostrcb->hcam.u.error64.fd_res_path[0], \
> -					&hostrcb->rp_buffer[0]),	\
> +					&hostrcb->rp_buffer[0],		\
> +					sizeof(hostrcb->rp_buffer)),	\
>  				__VA_ARGS__);				\
>  		} else {						\
>  			ipr_ra_err((hostrcb)->ioa_cfg,			\
> --
> 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] 10+ messages in thread

* Re: [PATCH] [SCSI] ipr: Fix stack overflow in ipr_format_resource_path
  2010-06-03  4:14   ` Wayne Boyer
@ 2010-06-03  4:20     ` Anton Blanchard
  -1 siblings, 0 replies; 10+ messages in thread
From: Anton Blanchard @ 2010-06-03  4:20 UTC (permalink / raw)
  To: Wayne Boyer; +Cc: brking, betabandido, linux-scsi, linuxppc-dev


Hi Wayne,

> We also saw a variation of this problem last week and I have an alternative
> patch that I'd prefer over this one.  I'll post the patch in a separate
> email.

Thanks. Can you get it into -stable too? As you can see users are hitting it.

Anton

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

* Re: [PATCH] [SCSI] ipr: Fix stack overflow in ipr_format_resource_path
@ 2010-06-03  4:20     ` Anton Blanchard
  0 siblings, 0 replies; 10+ messages in thread
From: Anton Blanchard @ 2010-06-03  4:20 UTC (permalink / raw)
  To: Wayne Boyer; +Cc: betabandido, linux-scsi, linuxppc-dev


Hi Wayne,

> We also saw a variation of this problem last week and I have an alternative
> patch that I'd prefer over this one.  I'll post the patch in a separate
> email.

Thanks. Can you get it into -stable too? As you can see users are hitting it.

Anton

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

* [PATCH 1/1] [SCSI] ipr: fix resource path display and formatting
  2010-06-02 11:16 ` Anton Blanchard
@ 2010-06-03 23:02   ` Wayne Boyer
  -1 siblings, 0 replies; 10+ messages in thread
From: Wayne Boyer @ 2010-06-03 23:02 UTC (permalink / raw)
  To: Anton Blanchard; +Cc: brking, betabandido, linux-scsi, linuxppc-dev

It was possible to overflow the buffer used to print out the formatted
version of the resource path.  The fix is to limit the number of
bytes that get formatted.

This patch also updates the ipr_show_resource_path function to display the
resource address for devices that are attached to adapters that don't
support resource paths.

Signed-off-by: Wayne Boyer <wayneb@linux.vnet.ibm.com>
---

James, this patch needs to go into rc-fixes.  It fixes an oops that is
currently being seen on Power.

---
 drivers/scsi/ipr.c |   51 +++++++++++++++++++++++++++++++++------------------
 drivers/scsi/ipr.h |    5 +++--
 2 files changed, 36 insertions(+), 20 deletions(-)

Index: b/drivers/scsi/ipr.c
===================================================================
--- a/drivers/scsi/ipr.c	2010-05-26 16:01:22.000000000 -0700
+++ b/drivers/scsi/ipr.c	2010-06-03 15:31:59.000000000 -0700
@@ -1129,20 +1129,22 @@ static int ipr_is_same_device(struct ipr
 }

 /**
- * ipr_format_resource_path - Format the resource path for printing.
+ * ipr_format_res_path - Format the resource path for printing.
  * @res_path:	resource path
  * @buf:	buffer
  *
  * Return value:
  * 	pointer to buffer
  **/
-static char *ipr_format_resource_path(u8 *res_path, char *buffer)
+static char *ipr_format_res_path(u8 *res_path, char *buffer, int len)
 {
 	int i;
+	char *p = buffer;

-	sprintf(buffer, "%02X", res_path[0]);
-	for (i=1; res_path[i] != 0xff; i++)
-		sprintf(buffer, "%s-%02X", buffer, res_path[i]);
+	res_path[0] = '\0';
+	p += snprintf(p, buffer + len - p, "%02X", res_path[0]);
+	for (i = 1; res_path[i] != 0xff && ((i * 3) < len); i++)
+		p += snprintf(p, buffer + len - p, "-%02X", res_path[i]);

 	return buffer;
 }
@@ -1187,7 +1189,8 @@ static void ipr_update_res_entry(struct 

 		if (res->sdev && new_path)
 			sdev_printk(KERN_INFO, res->sdev, "Resource path: %s\n",
-				    ipr_format_resource_path(&res->res_path[0], &buffer[0]));
+				    ipr_format_res_path(res->res_path, buffer,
+							sizeof(buffer)));
 	} else {
 		res->flags = cfgtew->u.cfgte->flags;
 		if (res->flags & IPR_IS_IOA_RESOURCE)
@@ -1573,7 +1576,8 @@ static void ipr_log_sis64_config_error(s
 		ipr_err_separator;

 		ipr_err("Device %d : %s", i + 1,
-			 ipr_format_resource_path(&dev_entry->res_path[0], &buffer[0]));
+			 ipr_format_res_path(dev_entry->res_path, buffer,
+					     sizeof(buffer)));
 		ipr_log_ext_vpd(&dev_entry->vpd);

 		ipr_err("-----New Device Information-----\n");
@@ -1919,13 +1923,14 @@ static void ipr_log64_fabric_path(struct

 			ipr_hcam_err(hostrcb, "%s %s: Resource Path=%s\n",
 				     path_active_desc[i].desc, path_state_desc[j].desc,
-				     ipr_format_resource_path(&fabric->res_path[0], &buffer[0]));
+				     ipr_format_res_path(fabric->res_path, buffer,
+							 sizeof(buffer)));
 			return;
 		}
 	}

 	ipr_err("Path state=%02X Resource Path=%s\n", path_state,
-		ipr_format_resource_path(&fabric->res_path[0], &buffer[0]));
+		ipr_format_res_path(fabric->res_path, buffer, sizeof(buffer)));
 }

 static const struct {
@@ -2066,7 +2071,8 @@ static void ipr_log64_path_elem(struct i

 			ipr_hcam_err(hostrcb, "%s %s: Resource Path=%s, Link rate=%s, WWN=%08X%08X\n",
 				     path_status_desc[j].desc, path_type_desc[i].desc,
-				     ipr_format_resource_path(&cfg->res_path[0], &buffer[0]),
+				     ipr_format_res_path(cfg->res_path, buffer,
+							 sizeof(buffer)),
 				     link_rate[cfg->link_rate & IPR_PHY_LINK_RATE_MASK],
 				     be32_to_cpu(cfg->wwid[0]), be32_to_cpu(cfg->wwid[1]));
 			return;
@@ -2074,7 +2080,7 @@ static void ipr_log64_path_elem(struct i
 	}
 	ipr_hcam_err(hostrcb, "Path element=%02X: Resource Path=%s, Link rate=%s "
 		     "WWN=%08X%08X\n", cfg->type_status,
-		     ipr_format_resource_path(&cfg->res_path[0], &buffer[0]),
+		     ipr_format_res_path(cfg->res_path, buffer, sizeof(buffer)),
 		     link_rate[cfg->link_rate & IPR_PHY_LINK_RATE_MASK],
 		     be32_to_cpu(cfg->wwid[0]), be32_to_cpu(cfg->wwid[1]));
 }
@@ -2139,7 +2145,7 @@ static void ipr_log_sis64_array_error(st

 	ipr_err("RAID %s Array Configuration: %s\n",
 		error->protection_level,
-		ipr_format_resource_path(&error->last_res_path[0], &buffer[0]));
+		ipr_format_res_path(error->last_res_path, buffer, sizeof(buffer)));

 	ipr_err_separator;

@@ -2160,9 +2166,11 @@ static void ipr_log_sis64_array_error(st
 		ipr_err("Array Member %d:\n", i);
 		ipr_log_ext_vpd(&array_entry->vpd);
 		ipr_err("Current Location: %s",
-			 ipr_format_resource_path(&array_entry->res_path[0], &buffer[0]));
+			 ipr_format_res_path(array_entry->res_path, buffer,
+					     sizeof(buffer)));
 		ipr_err("Expected Location: %s",
-			 ipr_format_resource_path(&array_entry->expected_res_path[0], &buffer[0]));
+			 ipr_format_res_path(array_entry->expected_res_path,
+					     buffer, sizeof(buffer)));

 		ipr_err_separator;
 	}
@@ -4076,7 +4084,8 @@ static struct device_attribute ipr_adapt
 };

 /**
- * ipr_show_resource_path - Show the resource path for this device.
+ * ipr_show_resource_path - Show the resource path or the resource address for
+ *			    this device.
  * @dev:	device struct
  * @buf:	buffer
  *
@@ -4094,9 +4103,14 @@ static ssize_t ipr_show_resource_path(st

 	spin_lock_irqsave(ioa_cfg->host->host_lock, lock_flags);
 	res = (struct ipr_resource_entry *)sdev->hostdata;
-	if (res)
+	if (res && ioa_cfg->sis64)
 		len = snprintf(buf, PAGE_SIZE, "%s\n",
-			       ipr_format_resource_path(&res->res_path[0], &buffer[0]));
+			       ipr_format_res_path(res->res_path, buffer,
+						   sizeof(buffer)));
+	else if (res)
+		len = snprintf(buf, PAGE_SIZE, "%d:%d:%d:%d\n", ioa_cfg->host->host_no,
+			       res->bus, res->target, res->lun);
+
 	spin_unlock_irqrestore(ioa_cfg->host->host_lock, lock_flags);
 	return len;
 }
@@ -4348,7 +4362,8 @@ static int ipr_slave_configure(struct sc
 			scsi_adjust_queue_depth(sdev, 0, sdev->host->cmd_per_lun);
 		if (ioa_cfg->sis64)
 			sdev_printk(KERN_INFO, sdev, "Resource path: %s\n",
-			            ipr_format_resource_path(&res->res_path[0], &buffer[0]));
+				    ipr_format_res_path(res->res_path, buffer,
+							sizeof(buffer)));
 		return 0;
 	}
 	spin_unlock_irqrestore(ioa_cfg->host->host_lock, lock_flags);
Index: b/drivers/scsi/ipr.h
===================================================================
--- a/drivers/scsi/ipr.h	2010-05-26 16:01:22.000000000 -0700
+++ b/drivers/scsi/ipr.h	2010-06-03 15:41:13.000000000 -0700
@@ -1684,8 +1684,9 @@ struct ipr_ucode_image_header {
 	if (ipr_is_device(hostrcb)) {					\
 		if ((hostrcb)->ioa_cfg->sis64) {			\
 			printk(KERN_ERR IPR_NAME ": %s: " fmt, 		\
-				ipr_format_resource_path(&hostrcb->hcam.u.error64.fd_res_path[0], \
-					&hostrcb->rp_buffer[0]),	\
+				ipr_format_res_path(hostrcb->hcam.u.error64.fd_res_path, \
+					hostrcb->rp_buffer,		\
+					sizeof(hostrcb->rp_buffer)),	\
 				__VA_ARGS__);				\
 		} else {						\
 			ipr_ra_err((hostrcb)->ioa_cfg,			\


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

* [PATCH 1/1] [SCSI] ipr: fix resource path display and formatting
@ 2010-06-03 23:02   ` Wayne Boyer
  0 siblings, 0 replies; 10+ messages in thread
From: Wayne Boyer @ 2010-06-03 23:02 UTC (permalink / raw)
  To: Anton Blanchard; +Cc: betabandido, linux-scsi, linuxppc-dev

It was possible to overflow the buffer used to print out the formatted
version of the resource path.  The fix is to limit the number of
bytes that get formatted.

This patch also updates the ipr_show_resource_path function to display the
resource address for devices that are attached to adapters that don't
support resource paths.

Signed-off-by: Wayne Boyer <wayneb@linux.vnet.ibm.com>
---

James, this patch needs to go into rc-fixes.  It fixes an oops that is
currently being seen on Power.

---
 drivers/scsi/ipr.c |   51 +++++++++++++++++++++++++++++++++------------------
 drivers/scsi/ipr.h |    5 +++--
 2 files changed, 36 insertions(+), 20 deletions(-)

Index: b/drivers/scsi/ipr.c
===================================================================
--- a/drivers/scsi/ipr.c	2010-05-26 16:01:22.000000000 -0700
+++ b/drivers/scsi/ipr.c	2010-06-03 15:31:59.000000000 -0700
@@ -1129,20 +1129,22 @@ static int ipr_is_same_device(struct ipr
 }

 /**
- * ipr_format_resource_path - Format the resource path for printing.
+ * ipr_format_res_path - Format the resource path for printing.
  * @res_path:	resource path
  * @buf:	buffer
  *
  * Return value:
  * 	pointer to buffer
  **/
-static char *ipr_format_resource_path(u8 *res_path, char *buffer)
+static char *ipr_format_res_path(u8 *res_path, char *buffer, int len)
 {
 	int i;
+	char *p = buffer;

-	sprintf(buffer, "%02X", res_path[0]);
-	for (i=1; res_path[i] != 0xff; i++)
-		sprintf(buffer, "%s-%02X", buffer, res_path[i]);
+	res_path[0] = '\0';
+	p += snprintf(p, buffer + len - p, "%02X", res_path[0]);
+	for (i = 1; res_path[i] != 0xff && ((i * 3) < len); i++)
+		p += snprintf(p, buffer + len - p, "-%02X", res_path[i]);

 	return buffer;
 }
@@ -1187,7 +1189,8 @@ static void ipr_update_res_entry(struct 

 		if (res->sdev && new_path)
 			sdev_printk(KERN_INFO, res->sdev, "Resource path: %s\n",
-				    ipr_format_resource_path(&res->res_path[0], &buffer[0]));
+				    ipr_format_res_path(res->res_path, buffer,
+							sizeof(buffer)));
 	} else {
 		res->flags = cfgtew->u.cfgte->flags;
 		if (res->flags & IPR_IS_IOA_RESOURCE)
@@ -1573,7 +1576,8 @@ static void ipr_log_sis64_config_error(s
 		ipr_err_separator;

 		ipr_err("Device %d : %s", i + 1,
-			 ipr_format_resource_path(&dev_entry->res_path[0], &buffer[0]));
+			 ipr_format_res_path(dev_entry->res_path, buffer,
+					     sizeof(buffer)));
 		ipr_log_ext_vpd(&dev_entry->vpd);

 		ipr_err("-----New Device Information-----\n");
@@ -1919,13 +1923,14 @@ static void ipr_log64_fabric_path(struct

 			ipr_hcam_err(hostrcb, "%s %s: Resource Path=%s\n",
 				     path_active_desc[i].desc, path_state_desc[j].desc,
-				     ipr_format_resource_path(&fabric->res_path[0], &buffer[0]));
+				     ipr_format_res_path(fabric->res_path, buffer,
+							 sizeof(buffer)));
 			return;
 		}
 	}

 	ipr_err("Path state=%02X Resource Path=%s\n", path_state,
-		ipr_format_resource_path(&fabric->res_path[0], &buffer[0]));
+		ipr_format_res_path(fabric->res_path, buffer, sizeof(buffer)));
 }

 static const struct {
@@ -2066,7 +2071,8 @@ static void ipr_log64_path_elem(struct i

 			ipr_hcam_err(hostrcb, "%s %s: Resource Path=%s, Link rate=%s, WWN=%08X%08X\n",
 				     path_status_desc[j].desc, path_type_desc[i].desc,
-				     ipr_format_resource_path(&cfg->res_path[0], &buffer[0]),
+				     ipr_format_res_path(cfg->res_path, buffer,
+							 sizeof(buffer)),
 				     link_rate[cfg->link_rate & IPR_PHY_LINK_RATE_MASK],
 				     be32_to_cpu(cfg->wwid[0]), be32_to_cpu(cfg->wwid[1]));
 			return;
@@ -2074,7 +2080,7 @@ static void ipr_log64_path_elem(struct i
 	}
 	ipr_hcam_err(hostrcb, "Path element=%02X: Resource Path=%s, Link rate=%s "
 		     "WWN=%08X%08X\n", cfg->type_status,
-		     ipr_format_resource_path(&cfg->res_path[0], &buffer[0]),
+		     ipr_format_res_path(cfg->res_path, buffer, sizeof(buffer)),
 		     link_rate[cfg->link_rate & IPR_PHY_LINK_RATE_MASK],
 		     be32_to_cpu(cfg->wwid[0]), be32_to_cpu(cfg->wwid[1]));
 }
@@ -2139,7 +2145,7 @@ static void ipr_log_sis64_array_error(st

 	ipr_err("RAID %s Array Configuration: %s\n",
 		error->protection_level,
-		ipr_format_resource_path(&error->last_res_path[0], &buffer[0]));
+		ipr_format_res_path(error->last_res_path, buffer, sizeof(buffer)));

 	ipr_err_separator;

@@ -2160,9 +2166,11 @@ static void ipr_log_sis64_array_error(st
 		ipr_err("Array Member %d:\n", i);
 		ipr_log_ext_vpd(&array_entry->vpd);
 		ipr_err("Current Location: %s",
-			 ipr_format_resource_path(&array_entry->res_path[0], &buffer[0]));
+			 ipr_format_res_path(array_entry->res_path, buffer,
+					     sizeof(buffer)));
 		ipr_err("Expected Location: %s",
-			 ipr_format_resource_path(&array_entry->expected_res_path[0], &buffer[0]));
+			 ipr_format_res_path(array_entry->expected_res_path,
+					     buffer, sizeof(buffer)));

 		ipr_err_separator;
 	}
@@ -4076,7 +4084,8 @@ static struct device_attribute ipr_adapt
 };

 /**
- * ipr_show_resource_path - Show the resource path for this device.
+ * ipr_show_resource_path - Show the resource path or the resource address for
+ *			    this device.
  * @dev:	device struct
  * @buf:	buffer
  *
@@ -4094,9 +4103,14 @@ static ssize_t ipr_show_resource_path(st

 	spin_lock_irqsave(ioa_cfg->host->host_lock, lock_flags);
 	res = (struct ipr_resource_entry *)sdev->hostdata;
-	if (res)
+	if (res && ioa_cfg->sis64)
 		len = snprintf(buf, PAGE_SIZE, "%s\n",
-			       ipr_format_resource_path(&res->res_path[0], &buffer[0]));
+			       ipr_format_res_path(res->res_path, buffer,
+						   sizeof(buffer)));
+	else if (res)
+		len = snprintf(buf, PAGE_SIZE, "%d:%d:%d:%d\n", ioa_cfg->host->host_no,
+			       res->bus, res->target, res->lun);
+
 	spin_unlock_irqrestore(ioa_cfg->host->host_lock, lock_flags);
 	return len;
 }
@@ -4348,7 +4362,8 @@ static int ipr_slave_configure(struct sc
 			scsi_adjust_queue_depth(sdev, 0, sdev->host->cmd_per_lun);
 		if (ioa_cfg->sis64)
 			sdev_printk(KERN_INFO, sdev, "Resource path: %s\n",
-			            ipr_format_resource_path(&res->res_path[0], &buffer[0]));
+				    ipr_format_res_path(res->res_path, buffer,
+							sizeof(buffer)));
 		return 0;
 	}
 	spin_unlock_irqrestore(ioa_cfg->host->host_lock, lock_flags);
Index: b/drivers/scsi/ipr.h
===================================================================
--- a/drivers/scsi/ipr.h	2010-05-26 16:01:22.000000000 -0700
+++ b/drivers/scsi/ipr.h	2010-06-03 15:41:13.000000000 -0700
@@ -1684,8 +1684,9 @@ struct ipr_ucode_image_header {
 	if (ipr_is_device(hostrcb)) {					\
 		if ((hostrcb)->ioa_cfg->sis64) {			\
 			printk(KERN_ERR IPR_NAME ": %s: " fmt, 		\
-				ipr_format_resource_path(&hostrcb->hcam.u.error64.fd_res_path[0], \
-					&hostrcb->rp_buffer[0]),	\
+				ipr_format_res_path(hostrcb->hcam.u.error64.fd_res_path, \
+					hostrcb->rp_buffer,		\
+					sizeof(hostrcb->rp_buffer)),	\
 				__VA_ARGS__);				\
 		} else {						\
 			ipr_ra_err((hostrcb)->ioa_cfg,			\

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

* Re: [PATCH 1/1] [SCSI] ipr: fix resource path display and formatting
  2010-06-03 23:02   ` Wayne Boyer
@ 2010-06-04 19:04     ` Brian King
  -1 siblings, 0 replies; 10+ messages in thread
From: Brian King @ 2010-06-04 19:04 UTC (permalink / raw)
  To: Wayne Boyer
  Cc: Anton Blanchard, brking, betabandido, linux-scsi, linuxppc-dev

Acked-by: Brian King <brking@linux.vnet.ibm.com>


On 06/03/2010 06:02 PM, Wayne Boyer wrote:
> It was possible to overflow the buffer used to print out the formatted
> version of the resource path.  The fix is to limit the number of
> bytes that get formatted.
> 
> This patch also updates the ipr_show_resource_path function to display the
> resource address for devices that are attached to adapters that don't
> support resource paths.
> 
> Signed-off-by: Wayne Boyer <wayneb@linux.vnet.ibm.com>
> ---
> 
> James, this patch needs to go into rc-fixes.  It fixes an oops that is
> currently being seen on Power.
> 
> ---
>  drivers/scsi/ipr.c |   51 +++++++++++++++++++++++++++++++++------------------
>  drivers/scsi/ipr.h |    5 +++--
>  2 files changed, 36 insertions(+), 20 deletions(-)
> 
> Index: b/drivers/scsi/ipr.c
> ===================================================================
> --- a/drivers/scsi/ipr.c	2010-05-26 16:01:22.000000000 -0700
> +++ b/drivers/scsi/ipr.c	2010-06-03 15:31:59.000000000 -0700
> @@ -1129,20 +1129,22 @@ static int ipr_is_same_device(struct ipr
>  }
> 
>  /**
> - * ipr_format_resource_path - Format the resource path for printing.
> + * ipr_format_res_path - Format the resource path for printing.
>   * @res_path:	resource path
>   * @buf:	buffer
>   *
>   * Return value:
>   * 	pointer to buffer
>   **/
> -static char *ipr_format_resource_path(u8 *res_path, char *buffer)
> +static char *ipr_format_res_path(u8 *res_path, char *buffer, int len)
>  {
>  	int i;
> +	char *p = buffer;
> 
> -	sprintf(buffer, "%02X", res_path[0]);
> -	for (i=1; res_path[i] != 0xff; i++)
> -		sprintf(buffer, "%s-%02X", buffer, res_path[i]);
> +	res_path[0] = '\0';
> +	p += snprintf(p, buffer + len - p, "%02X", res_path[0]);
> +	for (i = 1; res_path[i] != 0xff && ((i * 3) < len); i++)
> +		p += snprintf(p, buffer + len - p, "-%02X", res_path[i]);
> 
>  	return buffer;
>  }
> @@ -1187,7 +1189,8 @@ static void ipr_update_res_entry(struct 
> 
>  		if (res->sdev && new_path)
>  			sdev_printk(KERN_INFO, res->sdev, "Resource path: %s\n",
> -				    ipr_format_resource_path(&res->res_path[0], &buffer[0]));
> +				    ipr_format_res_path(res->res_path, buffer,
> +							sizeof(buffer)));
>  	} else {
>  		res->flags = cfgtew->u.cfgte->flags;
>  		if (res->flags & IPR_IS_IOA_RESOURCE)
> @@ -1573,7 +1576,8 @@ static void ipr_log_sis64_config_error(s
>  		ipr_err_separator;
> 
>  		ipr_err("Device %d : %s", i + 1,
> -			 ipr_format_resource_path(&dev_entry->res_path[0], &buffer[0]));
> +			 ipr_format_res_path(dev_entry->res_path, buffer,
> +					     sizeof(buffer)));
>  		ipr_log_ext_vpd(&dev_entry->vpd);
> 
>  		ipr_err("-----New Device Information-----\n");
> @@ -1919,13 +1923,14 @@ static void ipr_log64_fabric_path(struct
> 
>  			ipr_hcam_err(hostrcb, "%s %s: Resource Path=%s\n",
>  				     path_active_desc[i].desc, path_state_desc[j].desc,
> -				     ipr_format_resource_path(&fabric->res_path[0], &buffer[0]));
> +				     ipr_format_res_path(fabric->res_path, buffer,
> +							 sizeof(buffer)));
>  			return;
>  		}
>  	}
> 
>  	ipr_err("Path state=%02X Resource Path=%s\n", path_state,
> -		ipr_format_resource_path(&fabric->res_path[0], &buffer[0]));
> +		ipr_format_res_path(fabric->res_path, buffer, sizeof(buffer)));
>  }
> 
>  static const struct {
> @@ -2066,7 +2071,8 @@ static void ipr_log64_path_elem(struct i
> 
>  			ipr_hcam_err(hostrcb, "%s %s: Resource Path=%s, Link rate=%s, WWN=%08X%08X\n",
>  				     path_status_desc[j].desc, path_type_desc[i].desc,
> -				     ipr_format_resource_path(&cfg->res_path[0], &buffer[0]),
> +				     ipr_format_res_path(cfg->res_path, buffer,
> +							 sizeof(buffer)),
>  				     link_rate[cfg->link_rate & IPR_PHY_LINK_RATE_MASK],
>  				     be32_to_cpu(cfg->wwid[0]), be32_to_cpu(cfg->wwid[1]));
>  			return;
> @@ -2074,7 +2080,7 @@ static void ipr_log64_path_elem(struct i
>  	}
>  	ipr_hcam_err(hostrcb, "Path element=%02X: Resource Path=%s, Link rate=%s "
>  		     "WWN=%08X%08X\n", cfg->type_status,
> -		     ipr_format_resource_path(&cfg->res_path[0], &buffer[0]),
> +		     ipr_format_res_path(cfg->res_path, buffer, sizeof(buffer)),
>  		     link_rate[cfg->link_rate & IPR_PHY_LINK_RATE_MASK],
>  		     be32_to_cpu(cfg->wwid[0]), be32_to_cpu(cfg->wwid[1]));
>  }
> @@ -2139,7 +2145,7 @@ static void ipr_log_sis64_array_error(st
> 
>  	ipr_err("RAID %s Array Configuration: %s\n",
>  		error->protection_level,
> -		ipr_format_resource_path(&error->last_res_path[0], &buffer[0]));
> +		ipr_format_res_path(error->last_res_path, buffer, sizeof(buffer)));
> 
>  	ipr_err_separator;
> 
> @@ -2160,9 +2166,11 @@ static void ipr_log_sis64_array_error(st
>  		ipr_err("Array Member %d:\n", i);
>  		ipr_log_ext_vpd(&array_entry->vpd);
>  		ipr_err("Current Location: %s",
> -			 ipr_format_resource_path(&array_entry->res_path[0], &buffer[0]));
> +			 ipr_format_res_path(array_entry->res_path, buffer,
> +					     sizeof(buffer)));
>  		ipr_err("Expected Location: %s",
> -			 ipr_format_resource_path(&array_entry->expected_res_path[0], &buffer[0]));
> +			 ipr_format_res_path(array_entry->expected_res_path,
> +					     buffer, sizeof(buffer)));
> 
>  		ipr_err_separator;
>  	}
> @@ -4076,7 +4084,8 @@ static struct device_attribute ipr_adapt
>  };
> 
>  /**
> - * ipr_show_resource_path - Show the resource path for this device.
> + * ipr_show_resource_path - Show the resource path or the resource address for
> + *			    this device.
>   * @dev:	device struct
>   * @buf:	buffer
>   *
> @@ -4094,9 +4103,14 @@ static ssize_t ipr_show_resource_path(st
> 
>  	spin_lock_irqsave(ioa_cfg->host->host_lock, lock_flags);
>  	res = (struct ipr_resource_entry *)sdev->hostdata;
> -	if (res)
> +	if (res && ioa_cfg->sis64)
>  		len = snprintf(buf, PAGE_SIZE, "%s\n",
> -			       ipr_format_resource_path(&res->res_path[0], &buffer[0]));
> +			       ipr_format_res_path(res->res_path, buffer,
> +						   sizeof(buffer)));
> +	else if (res)
> +		len = snprintf(buf, PAGE_SIZE, "%d:%d:%d:%d\n", ioa_cfg->host->host_no,
> +			       res->bus, res->target, res->lun);
> +
>  	spin_unlock_irqrestore(ioa_cfg->host->host_lock, lock_flags);
>  	return len;
>  }
> @@ -4348,7 +4362,8 @@ static int ipr_slave_configure(struct sc
>  			scsi_adjust_queue_depth(sdev, 0, sdev->host->cmd_per_lun);
>  		if (ioa_cfg->sis64)
>  			sdev_printk(KERN_INFO, sdev, "Resource path: %s\n",
> -			            ipr_format_resource_path(&res->res_path[0], &buffer[0]));
> +				    ipr_format_res_path(res->res_path, buffer,
> +							sizeof(buffer)));
>  		return 0;
>  	}
>  	spin_unlock_irqrestore(ioa_cfg->host->host_lock, lock_flags);
> Index: b/drivers/scsi/ipr.h
> ===================================================================
> --- a/drivers/scsi/ipr.h	2010-05-26 16:01:22.000000000 -0700
> +++ b/drivers/scsi/ipr.h	2010-06-03 15:41:13.000000000 -0700
> @@ -1684,8 +1684,9 @@ struct ipr_ucode_image_header {
>  	if (ipr_is_device(hostrcb)) {					\
>  		if ((hostrcb)->ioa_cfg->sis64) {			\
>  			printk(KERN_ERR IPR_NAME ": %s: " fmt, 		\
> -				ipr_format_resource_path(&hostrcb->hcam.u.error64.fd_res_path[0], \
> -					&hostrcb->rp_buffer[0]),	\
> +				ipr_format_res_path(hostrcb->hcam.u.error64.fd_res_path, \
> +					hostrcb->rp_buffer,		\
> +					sizeof(hostrcb->rp_buffer)),	\
>  				__VA_ARGS__);				\
>  		} else {						\
>  			ipr_ra_err((hostrcb)->ioa_cfg,			\
> 


-- 
Brian King
Linux on Power Virtualization
IBM Linux Technology Center



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

* Re: [PATCH 1/1] [SCSI] ipr: fix resource path display and formatting
@ 2010-06-04 19:04     ` Brian King
  0 siblings, 0 replies; 10+ messages in thread
From: Brian King @ 2010-06-04 19:04 UTC (permalink / raw)
  To: Wayne Boyer; +Cc: betabandido, Anton Blanchard, linux-scsi, linuxppc-dev

Acked-by: Brian King <brking@linux.vnet.ibm.com>


On 06/03/2010 06:02 PM, Wayne Boyer wrote:
> It was possible to overflow the buffer used to print out the formatted
> version of the resource path.  The fix is to limit the number of
> bytes that get formatted.
> 
> This patch also updates the ipr_show_resource_path function to display the
> resource address for devices that are attached to adapters that don't
> support resource paths.
> 
> Signed-off-by: Wayne Boyer <wayneb@linux.vnet.ibm.com>
> ---
> 
> James, this patch needs to go into rc-fixes.  It fixes an oops that is
> currently being seen on Power.
> 
> ---
>  drivers/scsi/ipr.c |   51 +++++++++++++++++++++++++++++++++------------------
>  drivers/scsi/ipr.h |    5 +++--
>  2 files changed, 36 insertions(+), 20 deletions(-)
> 
> Index: b/drivers/scsi/ipr.c
> ===================================================================
> --- a/drivers/scsi/ipr.c	2010-05-26 16:01:22.000000000 -0700
> +++ b/drivers/scsi/ipr.c	2010-06-03 15:31:59.000000000 -0700
> @@ -1129,20 +1129,22 @@ static int ipr_is_same_device(struct ipr
>  }
> 
>  /**
> - * ipr_format_resource_path - Format the resource path for printing.
> + * ipr_format_res_path - Format the resource path for printing.
>   * @res_path:	resource path
>   * @buf:	buffer
>   *
>   * Return value:
>   * 	pointer to buffer
>   **/
> -static char *ipr_format_resource_path(u8 *res_path, char *buffer)
> +static char *ipr_format_res_path(u8 *res_path, char *buffer, int len)
>  {
>  	int i;
> +	char *p = buffer;
> 
> -	sprintf(buffer, "%02X", res_path[0]);
> -	for (i=1; res_path[i] != 0xff; i++)
> -		sprintf(buffer, "%s-%02X", buffer, res_path[i]);
> +	res_path[0] = '\0';
> +	p += snprintf(p, buffer + len - p, "%02X", res_path[0]);
> +	for (i = 1; res_path[i] != 0xff && ((i * 3) < len); i++)
> +		p += snprintf(p, buffer + len - p, "-%02X", res_path[i]);
> 
>  	return buffer;
>  }
> @@ -1187,7 +1189,8 @@ static void ipr_update_res_entry(struct 
> 
>  		if (res->sdev && new_path)
>  			sdev_printk(KERN_INFO, res->sdev, "Resource path: %s\n",
> -				    ipr_format_resource_path(&res->res_path[0], &buffer[0]));
> +				    ipr_format_res_path(res->res_path, buffer,
> +							sizeof(buffer)));
>  	} else {
>  		res->flags = cfgtew->u.cfgte->flags;
>  		if (res->flags & IPR_IS_IOA_RESOURCE)
> @@ -1573,7 +1576,8 @@ static void ipr_log_sis64_config_error(s
>  		ipr_err_separator;
> 
>  		ipr_err("Device %d : %s", i + 1,
> -			 ipr_format_resource_path(&dev_entry->res_path[0], &buffer[0]));
> +			 ipr_format_res_path(dev_entry->res_path, buffer,
> +					     sizeof(buffer)));
>  		ipr_log_ext_vpd(&dev_entry->vpd);
> 
>  		ipr_err("-----New Device Information-----\n");
> @@ -1919,13 +1923,14 @@ static void ipr_log64_fabric_path(struct
> 
>  			ipr_hcam_err(hostrcb, "%s %s: Resource Path=%s\n",
>  				     path_active_desc[i].desc, path_state_desc[j].desc,
> -				     ipr_format_resource_path(&fabric->res_path[0], &buffer[0]));
> +				     ipr_format_res_path(fabric->res_path, buffer,
> +							 sizeof(buffer)));
>  			return;
>  		}
>  	}
> 
>  	ipr_err("Path state=%02X Resource Path=%s\n", path_state,
> -		ipr_format_resource_path(&fabric->res_path[0], &buffer[0]));
> +		ipr_format_res_path(fabric->res_path, buffer, sizeof(buffer)));
>  }
> 
>  static const struct {
> @@ -2066,7 +2071,8 @@ static void ipr_log64_path_elem(struct i
> 
>  			ipr_hcam_err(hostrcb, "%s %s: Resource Path=%s, Link rate=%s, WWN=%08X%08X\n",
>  				     path_status_desc[j].desc, path_type_desc[i].desc,
> -				     ipr_format_resource_path(&cfg->res_path[0], &buffer[0]),
> +				     ipr_format_res_path(cfg->res_path, buffer,
> +							 sizeof(buffer)),
>  				     link_rate[cfg->link_rate & IPR_PHY_LINK_RATE_MASK],
>  				     be32_to_cpu(cfg->wwid[0]), be32_to_cpu(cfg->wwid[1]));
>  			return;
> @@ -2074,7 +2080,7 @@ static void ipr_log64_path_elem(struct i
>  	}
>  	ipr_hcam_err(hostrcb, "Path element=%02X: Resource Path=%s, Link rate=%s "
>  		     "WWN=%08X%08X\n", cfg->type_status,
> -		     ipr_format_resource_path(&cfg->res_path[0], &buffer[0]),
> +		     ipr_format_res_path(cfg->res_path, buffer, sizeof(buffer)),
>  		     link_rate[cfg->link_rate & IPR_PHY_LINK_RATE_MASK],
>  		     be32_to_cpu(cfg->wwid[0]), be32_to_cpu(cfg->wwid[1]));
>  }
> @@ -2139,7 +2145,7 @@ static void ipr_log_sis64_array_error(st
> 
>  	ipr_err("RAID %s Array Configuration: %s\n",
>  		error->protection_level,
> -		ipr_format_resource_path(&error->last_res_path[0], &buffer[0]));
> +		ipr_format_res_path(error->last_res_path, buffer, sizeof(buffer)));
> 
>  	ipr_err_separator;
> 
> @@ -2160,9 +2166,11 @@ static void ipr_log_sis64_array_error(st
>  		ipr_err("Array Member %d:\n", i);
>  		ipr_log_ext_vpd(&array_entry->vpd);
>  		ipr_err("Current Location: %s",
> -			 ipr_format_resource_path(&array_entry->res_path[0], &buffer[0]));
> +			 ipr_format_res_path(array_entry->res_path, buffer,
> +					     sizeof(buffer)));
>  		ipr_err("Expected Location: %s",
> -			 ipr_format_resource_path(&array_entry->expected_res_path[0], &buffer[0]));
> +			 ipr_format_res_path(array_entry->expected_res_path,
> +					     buffer, sizeof(buffer)));
> 
>  		ipr_err_separator;
>  	}
> @@ -4076,7 +4084,8 @@ static struct device_attribute ipr_adapt
>  };
> 
>  /**
> - * ipr_show_resource_path - Show the resource path for this device.
> + * ipr_show_resource_path - Show the resource path or the resource address for
> + *			    this device.
>   * @dev:	device struct
>   * @buf:	buffer
>   *
> @@ -4094,9 +4103,14 @@ static ssize_t ipr_show_resource_path(st
> 
>  	spin_lock_irqsave(ioa_cfg->host->host_lock, lock_flags);
>  	res = (struct ipr_resource_entry *)sdev->hostdata;
> -	if (res)
> +	if (res && ioa_cfg->sis64)
>  		len = snprintf(buf, PAGE_SIZE, "%s\n",
> -			       ipr_format_resource_path(&res->res_path[0], &buffer[0]));
> +			       ipr_format_res_path(res->res_path, buffer,
> +						   sizeof(buffer)));
> +	else if (res)
> +		len = snprintf(buf, PAGE_SIZE, "%d:%d:%d:%d\n", ioa_cfg->host->host_no,
> +			       res->bus, res->target, res->lun);
> +
>  	spin_unlock_irqrestore(ioa_cfg->host->host_lock, lock_flags);
>  	return len;
>  }
> @@ -4348,7 +4362,8 @@ static int ipr_slave_configure(struct sc
>  			scsi_adjust_queue_depth(sdev, 0, sdev->host->cmd_per_lun);
>  		if (ioa_cfg->sis64)
>  			sdev_printk(KERN_INFO, sdev, "Resource path: %s\n",
> -			            ipr_format_resource_path(&res->res_path[0], &buffer[0]));
> +				    ipr_format_res_path(res->res_path, buffer,
> +							sizeof(buffer)));
>  		return 0;
>  	}
>  	spin_unlock_irqrestore(ioa_cfg->host->host_lock, lock_flags);
> Index: b/drivers/scsi/ipr.h
> ===================================================================
> --- a/drivers/scsi/ipr.h	2010-05-26 16:01:22.000000000 -0700
> +++ b/drivers/scsi/ipr.h	2010-06-03 15:41:13.000000000 -0700
> @@ -1684,8 +1684,9 @@ struct ipr_ucode_image_header {
>  	if (ipr_is_device(hostrcb)) {					\
>  		if ((hostrcb)->ioa_cfg->sis64) {			\
>  			printk(KERN_ERR IPR_NAME ": %s: " fmt, 		\
> -				ipr_format_resource_path(&hostrcb->hcam.u.error64.fd_res_path[0], \
> -					&hostrcb->rp_buffer[0]),	\
> +				ipr_format_res_path(hostrcb->hcam.u.error64.fd_res_path, \
> +					hostrcb->rp_buffer,		\
> +					sizeof(hostrcb->rp_buffer)),	\
>  				__VA_ARGS__);				\
>  		} else {						\
>  			ipr_ra_err((hostrcb)->ioa_cfg,			\
> 


-- 
Brian King
Linux on Power Virtualization
IBM Linux Technology Center

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

end of thread, other threads:[~2010-06-04 19:04 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-06-02 11:16 [PATCH] [SCSI] ipr: Fix stack overflow in ipr_format_resource_path Anton Blanchard
2010-06-02 11:16 ` Anton Blanchard
2010-06-03  4:14 ` Wayne Boyer
2010-06-03  4:14   ` Wayne Boyer
2010-06-03  4:20   ` Anton Blanchard
2010-06-03  4:20     ` Anton Blanchard
2010-06-03 23:02 ` [PATCH 1/1] [SCSI] ipr: fix resource path display and formatting Wayne Boyer
2010-06-03 23:02   ` Wayne Boyer
2010-06-04 19:04   ` Brian King
2010-06-04 19:04     ` Brian King

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.