On Fri, May 23, 2014 at 11:37:03AM +0200, Borislav Petkov wrote:
> Date: Fri, 23 May 2014 11:37:03 +0200
> From: Borislav Petkov <bp@xxxxxxxxx>
> To: "Chen, Gong" <gong.chen@xxxxxxxxxxxxxxx>
> Cc: tony.luck@xxxxxxxxx, m.chehab@xxxxxxxxxxx, linux-acpi@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH 3/7 v4] CPER: Adjust code flow of some functions
> User-Agent: Mutt/1.5.23 (2014-03-12)
>
> On Thu, May 22, 2014 at 09:49:10PM -0400, Chen, Gong wrote:
> > If so, it has been there already. Maybe you should check patch
> > 5/7. I merge pa/pa_mask into pa_info as a whole to avoid too much
> > calculation/logic in trace.
>
> My bad, how did I miss that:
>
> > +static void __trace_mem_error(const uuid_le *fru_id, char *fru_text,
> > + u64 err_count, u32 severity,
> > + struct cper_sec_mem_err *mem)
> > +{
> > + u32 etype = ~0U;
> > + char pa_info[64];
> > + u8 n = 0;
> > +
> > + if (mem->validation_bits & CPER_MEM_VALID_ERROR_TYPE)
> > + etype = mem->error_type;
>
> More SNAFU: mem->error_type is u8 and you're saving it into a u32. What
> possible reason can you have for that?
OK, I will fix it.
>
> > +
> > + memset(pa_info, 0, 64);
> > + if (mem->validation_bits & CPER_MEM_VALID_PA)
> > + n = snprintf(pa_info, 63, "physical addr: 0x%016llx ",
> > + mem->physical_addr);
> > +
> > + if (mem->validation_bits & CPER_MEM_VALID_PA_MASK)
> > + snprintf(pa_info + n, 63 - n, "addr LSB: 0x%x ",
> > + (u8)__ffs64(mem->physical_addr_mask));
>
> So pa_info is 64 bytes!!! For what, a u64 and a u8? That's 9 bytes.
>
Oh, in my subconscious I am always afraid some kind of buffer overflow attach.
You are right, here it is obviously too wasteful. I can shrink it to 10
bytes. Please see my another reply for why I hope to use a string.
Attachment:
signature.asc
Description: Digital signature
