Messages in this thread | | | Date | Mon, 30 Apr 2012 13:47:02 -0300 | From | Mauro Carvalho Chehab <> | Subject | Re: [PATCH v2] edac_mc: Cleanup per-dimm_info debug messages |
| |
Em 30-04-2012 13:16, Joe Perches escreveu: > On Mon, 2012-04-30 at 12:02 -0300, Mauro Carvalho Chehab wrote: >> The edac_mc_alloc() routine allocates one dimm_info device for all >> possible memories, including the non-filled ones. The debug messages >> there are somewhat confusing. So, cleans them, by moving the code >> that prints the memory location to edac_mc, and using it on both >> edac_mc_sysfs and edac_mc. > [] >> diff --git a/drivers/edac/edac_mc.c b/drivers/edac/edac_mc.c > [] >> @@ -40,6 +40,25 @@ >> static DEFINE_MUTEX(mem_ctls_mutex); >> static LIST_HEAD(mc_devices); >> >> +unsigned edac_dimm_info_location(struct dimm_info *dimm, char *buf, >> + int len) >> +{ >> + struct mem_ctl_info *mci = dimm->mci; >> + int i, n, count = 0; >> + char *p = buf; >> + >> + for (i = 0; i < mci->n_layers; i++) { >> + n = snprintf(p, len, "%s %d ", >> + edac_layer_name[mci->layers[i].type], >> + dimm->location[i]); >> + p += n; >> + len -= n; >> + count += n; >> + } > > I believe this snprintf can be unsafe > when the buffer length len is exceeded. > > if len is negative, it's promoted to size_t > and continues to write into p.
Ok, I've changed it to unsigned at the new version and added a break to prevent this condition to happen. Version 3 with this fix and some other improvements were sent.
Yet, this condition will never happen, in practice, as the edac core currently supports only 3 layers.
Regards, Mauro.
| |