[PATCH] [SCSI] ipr: Fix stack overflow in ipr_format_resource_path
Wayne Boyer
wayneb at linux.vnet.ibm.com
Thu Jun 3 14:14:16 EST 2010
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 at 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 at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
More information about the Linuxppc-dev
mailing list