Re: [PATCH 0/2] EDAC, ghes: Fix use after free and add reference

[Robert Richter <rrichter@marvell.com>]

Hi Boris,

On 21.10.19 09:37:58, Borislav Petkov wrote:
> On Wed, Oct 16, 2019 at 08:50:41PM +0200, Borislav Petkov wrote:
> > Yeah, before we do this, lemme try to simplify the situation more. And
> > yeah, we don't need the mutex - we can use the spinlock only. But let's
> > get rid of the need to access the pvt in the IRQ handler. Yeah, we need
> > the *mci pointer but one can't have it all :)
> > 
> > Anyway, here's what I have, it is only build-tested. I wanna give it a
> > stern look tomorrow, on a clear head again:
> 
> And now I went and redid your patch ontop and thus completely got rid of
> ghes_pvt in favor of having one global ghes_mci pointer.

please do not do that. While we have atm only one ghes instance
including a single mci, we will need in the future multiple mci
instances to reflect the memory layout in sysfs. I sent already a
patch for this and am going to resent an updated version, see:

 https://lore.kernel.org/patchwork/patch/1093472/

> We access it only under the lock and we publish it in
> ghes_edac_register() only in the success case. Can't get any simpler
> than that.
> 
> Thoughts?

So, could we just keep all data in struct ghes_edac_pvt instead of
global data?

> I think it is a lot cleaner and straight-forward this way. Lemme know if
> you wanna run it on your setup and I'll push the two patches somewhere.

Could we please properly repost patches for review? James last one got
lost as an answer in this mail thread, but contains fundamental
changes of data structures and locking (while the both initially
posted patches contained reasonable oneliners).

I will start reviewing James' last patch now.

Thanks,

-Robert