[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH for-4.15] x86/ucode/amd: Fix OoB read in cpu_request_microcode()


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • Date: Wed, 10 Feb 2021 12:58:11 +0000
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=bmLS1XXA8+GOCyTy3cOL5oRSgP2O+b1POeq3ZqBmk+E=; b=EESNuh4msTMWI7USWUcpyV2w5CUyM6x36VL4D2946EQe9sSaR6i2V7wYEw/DfopN1YIBXVy5F+zNGaYTl7sbIwKHFawi9NJLbvGIcT1LzKiHT7WoOFN0L3WHjIGYZvqNkxuf+9HWjV3FwfckdqhVdyRhX/tWZFlnJ2ZQrFFOq3ew07ejLgx5NAOeqYvBy3pl7MaBMmv+M5h+hY0rqveWI1/AfXmkEhRa6ZRVoxLNi+aKYnrYWxPrGyBAX3fQAL6ioB2jXiuz55bqmmngG5+HEdzfHPl+zrYubGkaJSfbKDpdyWcNHOFlQKaflAVCRoosC6f0BxE+1qrXndRaV2Ucqg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=XUxq07yNxR/wdw/4xWZjwxPrXPpgNYI8+aFmZWTKavuF+bySgI7u9Fl23bHbSkzVSumlH/Lzl0/iiFfYFTTOzbvqTGSbcnAJLCRxHJHqL+J0u3VyW61U6CHQ+p8VI47lxU972iVMQCqJP2uAKOQIUlCCT02LLL678BH6gSorfTrfuo0zoW/rKsoeHoD/PBnznxo5d9yVby3v/BWoXbwNc8C0cJ4XyWBmTVUsgaFBYlz8wybPY/2v32iX1GZXKGgoQ3EbmhfwSCM1OUI77W+rkydfzwKUqB+d80GJxT39WsBeppkRNPJyFCnWRz2ffWutT1wYs32kiaZCtKLcqiNJyw==
  • Authentication-results: esa1.hc3370-68.iphmx.com; dkim=pass (signature verified) header.i=@citrix.onmicrosoft.com
  • Cc: Roger Pau Monné <roger.pau@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Ian Jackson <iwj@xxxxxxxxxxxxxx>, Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Wed, 10 Feb 2021 12:58:27 +0000
  • Ironport-sdr: mmI2HU8iDsZ+ZDvUYJqnDm7ognZ8bl2KD1kr2C4ujJEcmF7zEgzbdn//fcONBXPRqNjThHnhNp 2ZWHUnt2IQaSBt4UnxQi3HOPi/KRtvS2MvirXhcBmu7IHsssTWIk7KN70kV+llON7CbPXH91cL E6fp1Op9xdhVx/Fw5hAYesnWQIueCVCIqSRoAg2E484bG6UuVk1Mhks/8QI0UdG7rzpEho4uDZ FMnUUqrcaadKyrFMcyfVjPmpsfxpvpAUvNWKu5LCv+kUrdTgm1I5aa1eOQqkkA2iVEKQK3Y3cM 8X8=
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 10/02/2021 11:00, Jan Beulich wrote:
> On 10.02.2021 00:40, Andrew Cooper wrote:
>> verify_patch_size() is a maximum size check, and doesn't have a minimum 
>> bound.
>>
>> If the microcode container encodes a blob with a length less than 64 bytes,
>> the subsequent calls to microcode_fits()/compare_header() may read off the 
>> end
>> of the buffer.
>>
>> Fixes: 4de936a38a ("x86/ucode/amd: Rework parsing logic in 
>> cpu_request_microcode()")
>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Thanks.

>
>> --- a/xen/arch/x86/cpu/microcode/amd.c
>> +++ b/xen/arch/x86/cpu/microcode/amd.c
>> @@ -349,6 +349,7 @@ static struct microcode_patch 
>> *cpu_request_microcode(const void *buf, size_t siz
>>              if ( size < sizeof(*mc) ||
>>                   (mc = buf)->type != UCODE_UCODE_TYPE ||
>>                   size - sizeof(*mc) < mc->len ||
>> +                 mc->len < sizeof(struct microcode_patch) ||
> I was inclined to suggest to use <= here, but I guess a blob
> with 1 byte of data is as bogus as one with 0 bytes of data.

Yeah - its unfortunate.  On the Intel side, we've got a fairly rigorous
set of sanity checks we can perform on the blob, and on the AMD side we
appear to have nothing.

This at least prevents our minimal header checks from causing OoB accesses.

~Andrew



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.