From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2992590AbcB0Tog (ORCPT ); Sat, 27 Feb 2016 14:44:36 -0500 Received: from out1-smtp.messagingengine.com ([66.111.4.25]:60747 "EHLO out1-smtp.messagingengine.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751389AbcB0Toe (ORCPT ); Sat, 27 Feb 2016 14:44:34 -0500 X-Sasl-enc: DZ5WXiYnj2rY4H45sH8yIXDle4coXKLojBp3XyDtfVgr 1456602273 Date: Sat, 27 Feb 2016 16:44:30 -0300 From: Henrique de Moraes Holschuh To: Chris Bainbridge Cc: Borislav Petkov , x86@kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] x86/microcode: Change checksum to u32 Message-ID: <20160227194430.GC19798@khazad-dum.debian.net> References: <1456570907-5344-1-git-send-email-chris.bainbridge@gmail.com> <20160227175118.GC5261@pd.tnic> <20160227182329.GA4947@localhost> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20160227182329.GA4947@localhost> X-GPG-Fingerprint1: 4096R/39CB4807 C467 A717 507B BAFE D3C1 6092 0BD9 E811 39CB 4807 User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sat, 27 Feb 2016, Chris Bainbridge wrote: > > So what is ubsan complaining about? > > /* calculate the checksum */ > orig_sum = 0; > i = (MC_HEADER_SIZE + data_size) / DWSIZE; > while (i--) > orig_sum += ((int *)mc)[i]; > > The checksum is the additive sum of all the 32-bit values, but 32-bit > addition overflow is undefined for signed ints. For comparison the > checksum function from iucode-tool uses u32: > > intel_ucode_status_t intel_ucode_check_microcode(const void * const uc, > int strict) > { > ... > > uint32_t sum, orig_sum; > unsigned int i; > uint32_t *p; > > ... > > /* Calculate the checksum. We exclude the extended table as it > * also has to have a zero checksum, in order to get better > * coverage */ > orig_sum = 0; > i = (INTEL_UC_V1_HEADER_SIZE + data_size) / sizeof(uint32_t); > p = (uint32_t *)uc; > while (i--) { > orig_sum += *p; > p++; > } iucode-tool does it that way because: [Excerpt from the Intel 64 and IA‐32 Architectures Software Developer's Manual, Volume 3A: System Programming Guide]: 9.11.5 Microcode Update Checksum Each microcode update contains a DWORD checksum located in the update header. It is software’s responsibility to ensure that a microcode update is not corrupt. To check for a corrupt microcode update, software must perform a unsigned DWORD (32-bit) checksum of the microcode update. Even though some fields are signed, the checksum procedure treats all DWORDs as unsigned. Microcode updates with a header version equal to 00000001H must sum all DWORDs that comprise the microcode update. A valid checksum check will yield a value of 00000000H. Any other value indicates the microcode update is corrupt and should not be loaded." And "sum" will always overflow 32 bits on valid microcode, as the end result must be zero and has to warp-around for that to happen. Since the checksum algorithm really needs modulo-2^32 addition, it is best to be upfront about it... -- "One disk to rule them all, One disk to find them. One disk to bring them all and in the darkness grind them. In the Land of Redmond where the shadows lie." -- The Silicon Valley Tarot Henrique Holschuh