mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 0/2] EDAC/i5000: Fix the slot loop and re-enable the driver
@ 2026-11-11 23:25 Andrew Luebke
  2026-11-11 23:25 ` [PATCH 1/2] i5000_edac: fix slot number passed to determine_mtr() Andrew Luebke
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Andrew Luebke @ 2026-11-11 23:25 UTC (permalink / raw)
  To: Borislav Petkov, Tony Luck
  Cc: Aristeu Rozanski, Mauro Carvalho Chehab, linux-edac,
	linux-kernel, Andrew Luebke

In 2022 i5000_edac was marked BROKEN instead of taking Aristeu's fix for
an out-of-bounds read in i5000_init_csrows(). Nobody had the hardware to
check that the driver handles both single- and dual-ranked DIMMs [1].

I tested on an HP xw8400 (5000X, four channels) with v7.4-rc2, KASAN,
UBSAN bounds and EDAC debug. Without patch 1, UBSAN reports at driver
load:

  UBSAN: array-index-out-of-bounds in drivers/edac/i5000_edac.c:959:20
  index 4 is out of range for type 'u16 [4]'

and the same at line 961. With it there are no reports, and all eight
slots (4 x 2 GB 2Rx4 in slot 0, 4 x 512 MB 1Rx8 in slot 1, both branches)
show up at the right branch/channel/slot with the right size and rank,
10240 MB in total. On 7.2.14 the same holds, also with 4 x 1 GB 2Rx8 in
slot 1 (12288 MB). A two-pass memtester run with the driver polling (two
DIMMs) saw no errors.

Not tested: two-channel boards (5000V/5000Z) and error reporting, as no
memory errors occurred.

Patch 1 is Aristeu's patch as posted, plus my tags. Patch 2 reverts the
BROKEN marking.

6.1, 5.15 and 5.10 do not have the BROKEN marking, so the out-of-bounds
read is live there. Patch 1 applies to them as is.

Analysis and test kernels were prepared with LLM help; the tests ran on my
hardware.

[1] https://lore.kernel.org/all/YyxpbRkuLPIcW1Om@zn.tnic/

Andrew Luebke (1):
  Revert "EDAC/i5000: Mark as BROKEN"

Aristeu Rozanski (1):
  i5000_edac: fix slot number passed to determine_mtr()

 drivers/edac/Kconfig      | 1 -
 drivers/edac/i5000_edac.c | 4 +---
 2 files changed, 1 insertion(+), 4 deletions(-)


base-commit: 7475ea8b58616b33c04afb976cf0fbaad9dd63fb
-- 
2.47.3


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/2] i5000_edac: fix slot number passed to determine_mtr()
  2026-11-11 23:25 [PATCH 0/2] EDAC/i5000: Fix the slot loop and re-enable the driver Andrew Luebke
@ 2026-11-11 23:25 ` Andrew Luebke
  2026-11-11 23:25 ` [PATCH 2/2] Revert "EDAC/i5000: Mark as BROKEN" Andrew Luebke
  2026-11-12  0:17 ` [PATCH 0/2] EDAC/i5000: Fix the slot loop and re-enable the driver Borislav Petkov
  2 siblings, 0 replies; 5+ messages in thread
From: Andrew Luebke @ 2026-11-11 23:25 UTC (permalink / raw)
  To: Borislav Petkov, Tony Luck
  Cc: Aristeu Rozanski, Mauro Carvalho Chehab, linux-edac,
	linux-kernel, Andrew Luebke

From: Aristeu Rozanski <aris@redhat.com>

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>
Link: https://lore.kernel.org/all/20220921181009.oxytvicy6sry6it7@redhat.com/
Cc: stable@vger.kernel.org
Tested-by: Andrew Luebke <andrew.luebke@gmail.com>
Signed-off-by: Andrew Luebke <andrew.luebke@gmail.com>
---
 drivers/edac/i5000_edac.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/edac/i5000_edac.c b/drivers/edac/i5000_edac.c
index c0faf55f7812..342b69909600 100644
--- a/drivers/edac/i5000_edac.c
+++ b/drivers/edac/i5000_edac.c
@@ -1253,14 +1253,12 @@ static int i5000_init_csrows(struct mem_ctl_info *mci)
 	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;
 
 	empty = 1;		/* Assume NO memory */
 
@@ -1271,7 +1269,7 @@ static int i5000_init_csrows(struct mem_ctl_info *mci)
 	 * to map the dimms. A good cleanup would be to remove this array,
 	 * and do a loop here with branch, channel, slot
 	 */
-	for (slot = 0; slot < max_csrows; slot++) {
+	for (slot = 0; slot < pvt->maxdimmperch; slot++) {
 		for (channel = 0; channel < pvt->maxch; channel++) {
 
 			mtr = determine_mtr(pvt, slot, channel);
-- 
2.47.3


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 2/2] Revert "EDAC/i5000: Mark as BROKEN"
  2026-11-11 23:25 [PATCH 0/2] EDAC/i5000: Fix the slot loop and re-enable the driver Andrew Luebke
  2026-11-11 23:25 ` [PATCH 1/2] i5000_edac: fix slot number passed to determine_mtr() Andrew Luebke
@ 2026-11-11 23:25 ` Andrew Luebke
  2026-11-12  0:17 ` [PATCH 0/2] EDAC/i5000: Fix the slot loop and re-enable the driver Borislav Petkov
  2 siblings, 0 replies; 5+ messages in thread
From: Andrew Luebke @ 2026-11-11 23:25 UTC (permalink / raw)
  To: Borislav Petkov, Tony Luck
  Cc: Aristeu Rozanski, Mauro Carvalho Chehab, linux-edac,
	linux-kernel, Andrew Luebke

This reverts commit 7556419180a3 ("EDAC/i5000: Mark as BROKEN").

The driver was marked BROKEN, instead of taking the slot loop fix,
because nobody had the hardware to check it with single- and dual-ranked
DIMMs [1]. With the previous patch it loads on an HP xw8400 (5000X) with
all eight slots populated, reports single- and dual-ranked FB-DIMMs with
the right size at the right branch, channel and slot, and stays clean
under KASAN and UBSAN bounds checking.

Two-channel boards (5000V/5000Z) and error reporting are not tested.

[1] https://lore.kernel.org/all/YyxpbRkuLPIcW1Om@zn.tnic/

Assisted-by: LLM
Signed-off-by: Andrew Luebke <andrew.luebke@gmail.com>
---
 drivers/edac/Kconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
index 4c70729b0d3a..63f263c4263d 100644
--- a/drivers/edac/Kconfig
+++ b/drivers/edac/Kconfig
@@ -218,7 +218,6 @@ config EDAC_I82860
 config EDAC_I5000
 	tristate "Intel Greencreek/Blackford chipset"
 	depends on X86 && PCI
-	depends on BROKEN
 	help
 	  Support for error detection and correction the Intel
 	  Greekcreek/Blackford chipsets.
-- 
2.47.3


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 0/2] EDAC/i5000: Fix the slot loop and re-enable the driver
  2026-11-11 23:25 [PATCH 0/2] EDAC/i5000: Fix the slot loop and re-enable the driver Andrew Luebke
  2026-11-11 23:25 ` [PATCH 1/2] i5000_edac: fix slot number passed to determine_mtr() Andrew Luebke
  2026-11-11 23:25 ` [PATCH 2/2] Revert "EDAC/i5000: Mark as BROKEN" Andrew Luebke
@ 2026-11-12  0:17 ` Borislav Petkov
  2026-11-12 16:58   ` Andrew Luebke
  2 siblings, 1 reply; 5+ messages in thread
From: Borislav Petkov @ 2026-11-12  0:17 UTC (permalink / raw)
  To: Andrew Luebke
  Cc: Tony Luck, Aristeu Rozanski, Mauro Carvalho Chehab, linux-edac,
	linux-kernel

On Wed, Nov 11, 2026 at 03:25:27PM -0800, Andrew Luebke wrote:
> Analysis and test kernels were prepared with LLM help; the tests ran on my
> hardware.

Just out of curiosity, was all the energy you've spent on fixing this
- especially the AI tokens - worth the effort?

This chip is documented on something called the retro web:
https://theretroweb.com/chips/317 and it says it is from 2006 and it sports
DDR2 memory. So that is retro alright.

The HP xw8400 has a whopping quad-core Xeon in it.

But whatever...

Now, if you really wanna do this, can I jot you down as the one who will be
testing any future patches for that driver?

-- 
Regards/Gruss,
    Boris.

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 0/2] EDAC/i5000: Fix the slot loop and re-enable the driver
  2026-11-12  0:17 ` [PATCH 0/2] EDAC/i5000: Fix the slot loop and re-enable the driver Borislav Petkov
@ 2026-11-12 16:58   ` Andrew Luebke
  0 siblings, 0 replies; 5+ messages in thread
From: Andrew Luebke @ 2026-11-12 16:58 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Tony Luck, Aristeu Rozanski, Mauro Carvalho Chehab, linux-edac,
	linux-kernel

 > was all the energy you've spent on fixing this
 > - especially the AI tokens - worth the effort?

For me, yes. I still test and repair old medical equipment built on the
xw8400 and xw6400 (as well as even older systems), and FB-DIMMs are the
part that fails. Having EDAC name the slot beats swapping sticks.

 > can I jot you down as the one who will be testing any future
 > patches for that driver?

Yes, please do. I have the hardware on the bench, with single- and
dual-ranked FB-DIMMs, and can test patches on request.

On 11/11/26 4:17 PM, Borislav Petkov wrote:
> On Wed, Nov 11, 2026 at 03:25:27PM -0800, Andrew Luebke wrote:
>> Analysis and test kernels were prepared with LLM help; the tests ran on my
>> hardware.
> Just out of curiosity, was all the energy you've spent on fixing this
> - especially the AI tokens - worth the effort?
>
> This chip is documented on something called the retro web:
> https://theretroweb.com/chips/317 and it says it is from 2006 and it sports
> DDR2 memory. So that is retro alright.
>
> The HP xw8400 has a whopping quad-core Xeon in it.
>
> But whatever...
>
> Now, if you really wanna do this, can I jot you down as the one who will be
> testing any future patches for that driver?
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-11-12 16:58 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-11 23:25 [PATCH 0/2] EDAC/i5000: Fix the slot loop and re-enable the driver Andrew Luebke
2026-11-11 23:25 ` [PATCH 1/2] i5000_edac: fix slot number passed to determine_mtr() Andrew Luebke
2026-11-11 23:25 ` [PATCH 2/2] Revert "EDAC/i5000: Mark as BROKEN" Andrew Luebke
2026-11-12  0:17 ` [PATCH 0/2] EDAC/i5000: Fix the slot loop and re-enable the driver Borislav Petkov
2026-11-12 16:58   ` Andrew Luebke

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®