Re: [RESEND PATCH] i5000_edac: fix slot number passed to determine_mtr()

[Borislav Petkov <bp@alien8.de>]

On Wed, Sep 21, 2022 at 02:10:09PM -0400, Aristeu Rozanski wrote:
> When the logic mapping branch/slot/channel was reworked back in
> 64e1fdaf55d6 ("i5000_edac: Fix the logic that retrieves memory information")
> i5000_init_csrows() was not updated and kept passing twice the number
> of slots to determine_mtr(), which leads to acessing past the end of
> i5000_pvt.b1_mtr[]. This was found by KASAN.
> 
> Fixes: 64e1fdaf55d6 ("i5000_edac: Fix the logic that retrieves memory information")
> Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
> Cc: Borislav Petkov <bp@suse.de>
> Signed-off-by: Aristeu Rozanski <aris@redhat.com>
> 
> ---
>  drivers/edac/i5000_edac.c |    4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> --- linus-2.6.orig/drivers/edac/i5000_edac.c	2022-07-25 15:26:40.093989879 -0400
> +++ linus-2.6/drivers/edac/i5000_edac.c	2022-07-26 14:32:23.644694778 -0400
> @@ -1249,14 +1249,12 @@ static int i5000_init_csrows(struct mem_
>  	struct i5000_pvt *pvt;
>  	struct dimm_info *dimm;
>  	int empty;
> -	int max_csrows;
>  	int mtr;
>  	int csrow_megs;
>  	int channel;
>  	int slot;
>  
>  	pvt = mci->pvt_info;
> -	max_csrows = pvt->maxdimmperch * 2;

So it looks to me like back then, the number of CS rows was twice the
DIMMs per channel, implying two CS rows per DIMM, I guess dual-ranked
DIMMs or so.

Now you're saying the number CS rows is the number of DIMMs. Which
means, single-ranked DIMMs?

But all this is broken as it needs to deal with both single-ranked and
dual-ranked DIMMs elegantly and not assume any DIMM types...

Do we have hardware where we can test this or can we declare this hw for
dead?

Because

https://www.intel.com/content/www/us/en/products/sku/27746/intel-5000p-memory-controller/specifications.html

says launch date Q1 2006 which is a whole another era in IT years.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette