Re: [PATCH 3/7 v4] CPER: Adjust code flow of some functions

["Chen, Gong" <gong.chen@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 1970 bytes --]

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@alien8.de>
> To: "Chen, Gong" <gong.chen@linux.intel.com>
> Cc: tony.luck@intel.com, m.chehab@samsung.com, linux-acpi@vger.kernel.org
> 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 #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]