Re: [PATCH v2] edac_mc: Cleanup per-dimm_info debug messages

From: Mauro Carvalho Chehab
Date: Mon Apr 30 2012 - 12:47:08 EST


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