[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 4/4] x86/microcode_amd: Fail attempts to load a 0-length microcode blob.
On 24/09/13 15:26, Boris Ostrovsky wrote: > On 09/24/2013 09:19 AM, Jan Beulich wrote: >>>>> On 24.09.13 at 14:10, Andrew Cooper <andrew.cooper3@xxxxxxxxxx> >>>>> wrote: >>> Coverity ID: 1055319 >>> >>> Coverity identified that when passed a microcode header with a >>> length field of >>> 0, get_ucode_from_buffer_amd() would end up calling memcpy(NULL, >>> data, 0) >>> which is undefined behaviour. >> I think that's at least questionable: memcpy(..., 0) can hardly be >> anything but a no-op, no matter whether either of the two pointers >> in fact is a NULL one. > > I think what this patch is trying to prevent is passing NULL pointer > to memcpy, > not length being zero (if you follow the logic in > get_ucode_from_buffer_amd() you > will see that destination buffer may not be allocated when length is > zero). > > AMD APM says > > REP. The REP prefix repeats its associated string instruction the > number > of times specified in the counter register (rCX). It terminates the > repetition when the value in rCX reaches 0. > > So presumably rCX is checked before rDI and memcpy would indeed be a nop. > > Still, I think in this particular case (destination can be NULL) the > patch is > a good thing since we usually (often) check before passing NULL > pointer to > routines. Alternatively, we can check 'if (mc_amd->mpb != NULL)'. > > -boris That may not work as expected. The subtalty here is that we only allocate memory for mc_amd->mpb if mc_amd->mpb_size is less than mpbuf->len. mc_amd->mpb is unconditionally NULL before the first allocation, and mc_amd->mpb_size is 0 (from the caller) Therefore, a user passing 0 for mpbuf->len causes the routine to choose not to allocate any memory, then copy 0 bytes into it. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |