From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1756713AbcB0Rv0 (ORCPT <rfc822;w@1wt.eu>);
	Sat, 27 Feb 2016 12:51:26 -0500
Received: from mail.skyhub.de ([78.46.96.112]:40623 "EHLO mail.skyhub.de"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1756553AbcB0Rv0 (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
	Sat, 27 Feb 2016 12:51:26 -0500
Date: Sat, 27 Feb 2016 18:51:18 +0100
From: Borislav Petkov <bp@alien8.de>
To: Chris Bainbridge <chris.bainbridge@gmail.com>
Cc: x86@kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] x86/microcode: Change checksum to u32
Message-ID: <20160227175118.GC5261@pd.tnic>
References: <1456570907-5344-1-git-send-email-chris.bainbridge@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <1456570907-5344-1-git-send-email-chris.bainbridge@gmail.com>
User-Agent: Mutt/1.5.24 (2015-08-30)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Sat, Feb 27, 2016 at 11:01:47AM +0000, Chris Bainbridge wrote:
> Checksum should be unsigned 32-bit otherwise the calculation overflows
> resulting in undefined behaviour:
> 
> [    0.000000] ================================================================================
> [    0.000000] UBSAN: Undefined behaviour in arch/x86/kernel/cpu/microcode/intel_lib.c:105:12
> [    0.000000] signed integer overflow:
> [    0.000000] -1500151068 + -2125470173 cannot be represented in type 'int'

So I've been staring at this error message for a while now and I'm
having hard time understanding what it is telling me. Let's look at all
three ints:

* sum is computed here:

                sum = orig_sum
                        - (mc_header->sig + mc_header->pf + mc_header->cksum)
                        + (ext_sig->sig + ext_sig->pf + ext_sig->cksum);

and checks mc_header against each extended signature. If it overflows, we abort:

                if (sum) {
                        if (print_err)
                                pr_err("aborting, bad checksum\n");


* orig_sum in the above sum can only be 0:

        if (orig_sum) {
                if (print_err)
                        pr_err("aborting, bad checksum\n");
                return -EINVAL;

and it adds a bunch of integers which can overflow, sure, but if it
overflows, we exit early.

* and

	ext_sigcount = ext_header->count;

so it is some count of extended signatures.

So what is ubsan complaining about?

/me is confused.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.