x86_64 NUMA : Bug correction in populate_memnodemap()

PATCH In review Eric Dumazet 2 participants 4 messages Dec 7, 2005 – Dec 12, 2005 1 message not in the archive

lore

Share & tools

lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

Eric Dumazet

author

2005-12-07 15:35 UTC #1 lore raw

PATCH 1/1 x86_64 NUMA : Bug correction in populate_memnodemap()

As reported by Keith Mannthey, there are problems in populate_memnodemap()

The bug was that the compute_hash_shift() was returning 31, with incorrect initialization of memnodemap[]

To correct the bug, we must use (1UL << shift) instead of (1 << shift) to avoid an integer overflow, and we must check that shift < 64 to avoid an infinite loop.

Signed-off-by Eric Dumazet <[email protected]>

arch/x86_64/mm/numa.c +3 -1

@@ -53,6 +53,8 @@
 	int res = -1;
 	unsigned long addr, end;
 
+	if (shift >= 64)
+		return -1;
 	memset(memnodemap, 0xff, sizeof(memnodemap));
 	for (i = 0; i < numnodes; i++) {
 		addr = nodes[i].start;

@@ -65,7 +67,7 @@
 			if (memnodemap[addr >> shift] != 0xff)
 				return -1;
 			memnodemap[addr >> shift] = i;
-			addr += (1 << shift);
+                       addr += (1UL << shift);
 		} while (addr < end);
 		res = 1;
 	}

Andi Kleen

2005-12-07 17:46 UTC #2 lore raw

Quoted 8 lines · Eric Dumazet

As reported by Keith Mannthey, there are problems in populate_memnodemap()
The bug was that the compute_hash_shift() was returning 31, with incorrect initialization of memnodemap[]
To correct the bug, we must use (1UL << shift) instead of (1 << shift) to avoid an integer overflow, and we must check that shift < 64 to avoid an infinite loop.

Thanks looks good.

I guess we'll need that for 2.6.15.

-Andi

Andi Kleen

2005-12-11 18:28 UTC #3 lore raw

In reply to Eric Dumazet

Quoted 8 lines · Eric Dumazet

As reported by Keith Mannthey, there are problems in populate_memnodemap()
The bug was that the compute_hash_shift() was returning 31, with incorrect initialization of memnodemap[]
To correct the bug, we must use (1UL << shift) instead of (1 << shift) to avoid an integer overflow, and we must check that shift < 64 to avoid an infinite loop.

It's already merged, no need to resubmit. P.S.: It would be easier to merge if you didn't use attachments.

-Andi

Eric Dumazet

author

2005-12-12 08:35 UTC #4 lore raw

Quoted 11 lines · Andi Kleen

Quoted 5 lines · Eric Dumazet
As reported by Keith Mannthey, there are problems in populate_memnodemap()
The bug was that the compute_hash_shift() was returning 31, with incorrect initialization of memnodemap[]
To correct the bug, we must use (1UL << shift) instead of (1 << shift) to avoid an integer overflow, and we must check that shift < 64 to avoid an infinite loop.
It's already merged, no need to resubmit. P.S.: It would be easier to merge if you didn't use attachments.

Hi Andi

AFAIK I posted the patch on linux-kernel only once... and once privatly to Keith because it seems he missed the linux-kernel post. I'm confused by your answer.

Eric