Re: [Xen-devel] Hazardous memset/memcpy idiom (was Re: [PATCH] x86: fix null pointer dereference in intel_get_extended_msrs())

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2013-02-27 at 08:50 +0000, Jan Beulich wrote:
> >>> On 26.02.13 at 18:08, Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx> wrote:
> > Xi Wang writes ("[Xen-devel] [PATCH] x86: fix null pointer dereference in 
> > intel_get_extended_msrs()"):
> >> `memset(&mc_ext, 0, ...)' leads to a buffer overflow and a subsequent
> >> null pointer dereference.  Replace `&mc_ext' with `mc_ext'.
> > 
> > Really I think we shouldn't be writing out these kind of memsets.
> > They're too error-prone.  We should have a macro, perhaps like this:
> > 
> >   #define FILLZERO(object) memset(&(object), 0, sizeof(object))
> > 
> > Likewise a copy macro.
> 
> Unfortunately that wouldn't be usable in all cases (arrays passed
> to functions), and hence I'm not really confident this would be a
> good move. But yes, I considered what you suggest a number of
> times already, but dropped the idea each time because of its lack
> of general applicability.
> 
> I'm afraid we just have to live with the bad side effects of the
> power C provides.

In this specific case perhaps x86_mcinfo_reserve() should zero the
buffer instead of each caller having to do the right thing? Most callers
have a memset, a couple immediately fill in the buffer instead of
zeroing it, but this isn't a hot path, is it?

Ian.



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel