Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Thu, 17 May 2018 00:46:41 +0200
From:   Borislav Petkov <bp@suse.de>
To:     Johannes Hirte <johannes.hirte@datenkhaos.de>,
        "Ghannam, Yazen" <Yazen.Ghannam@amd.com>
Cc:     "linux-edac@vger.kernel.org" <linux-edac@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "tony.luck@intel.com" <tony.luck@intel.com>,
        "x86@kernel.org" <x86@kernel.org>
Subject: Re: [PATCH 3/3] x86/MCE/AMD: Get address from already initialized
 block
Message-ID: <20180516224641.GA31929@pd.tnic>
References: <20180201184813.82253-1-Yazen.Ghannam@amd.com>
 <20180201184813.82253-3-Yazen.Ghannam@amd.com>
 <20180414004230.GA2033@probook>
 <20180416115624.GA1543@probook>
 <CY4PR12MB1557FB8EB5C17E55A9B405D8F8B70@CY4PR12MB1557.namprd12.prod.outlook.com>
 <20180515093953.GA1746@probook>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20180515093953.GA1746@probook>
User-Agent: Mutt/1.9.3 (2018-01-21)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Tue, May 15, 2018 at 11:39:54AM +0200, Johannes Hirte wrote:
> The out-of-bound access happens in get_block_address:
> 
> 	if (bankp && bankp->blocks) {
> 		struct threshold_block *blockp blockp = &bankp->blocks[block];
> 
> with block=1. This doesn't exists. I don't even find any array here.
> There is a linked list, created in allocate_threshold_blocks. On my
> system I get 17 lists with one element each.

Yes, what a mess this is. ;-\

There's no such thing as ->blocks[block] array. We assign simply the
threshold_block to it in allocate_threshold_blocks:

	per_cpu(threshold_banks, cpu)[bank]->blocks = b;

And I can't say the design of this thing is really friendly but it is
still no excuse that I missed that during review. Grrr.

So, Yazen, what really needs to happen here is to iterate the
bank->blocks->miscj list to find the block you're looking for and return
its address, the opposite to this here:

        if (per_cpu(threshold_banks, cpu)[bank]->blocks) {
                list_add(&b->miscj,
                         &per_cpu(threshold_banks, cpu)[bank]->blocks->miscj);
        } else {
                per_cpu(threshold_banks, cpu)[bank]->blocks = b;
        }

and don't forget to look at ->blocks itself.

And then you need to make sure that searching for block addresses still
works when resuming from suspend so that you can avoid the RDMSR IPIs.

Ok?

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
--