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@xxxxxxxxx> > --- > > 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@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
