Skip to contentSkip to conversation

Fix an error in copy_page_range

PATCH In review Zou Nan hai 2 participants 2 messages Jan 20, 2005 – Jan 22, 2005

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Fix an error in copy_page_range

Hi,

There is a bug in copy_page_range in current 2.6.11-rc1 with 4 level page table change. copy_page_range do a continue without adding pgds and addr when pgd_none(*src_pgd) or pgd_bad(*src_pgd).

I think it's wrong in logic, copy_page_range will run into infinite loop when when pgd_none(*src_pgd) or pgd_bad(*src_pgd).

Although maybe this bug does not break anything currently...,

mm/memory.c +3 -2
@@ -442,17 +442,18 @@ int copy_page_range(struct mm_struct *ds
 		if (next > end || next <= addr)
 			next = end;
 		if (pgd_none(*src_pgd))
-			continue;
+			goto next_pgd;
 		if (pgd_bad(*src_pgd)) {
 			pgd_ERROR(*src_pgd);
 			pgd_clear(src_pgd);
-			continue;
+			goto next_pgd;
 		}
 		err = copy_pud_range(dst, src, dst_pgd, src_pgd,
 							vma, addr, next);
 		if (err)
 			break;
 
+next_pgd:
 		src_pgd++;
 		dst_pgd++;
 		addr = next;
#1 lore raw
Quoted 30 lines · Zou Nan hai

Hi,

There is a bug in copy_page_range in current 2.6.11-rc1 with 4 level page table change. copy_page_range do a continue without adding pgds and addr when pgd_none(*src_pgd) or pgd_bad(*src_pgd).

I think it's wrong in logic, copy_page_range will run into infinite loop when when pgd_none(*src_pgd) or pgd_bad(*src_pgd).

Although maybe this bug does not break anything currently...,

mm/memory.c +2 -2
@@ -442,17 +442,18 @@ int copy_page_range(struct mm_struct *ds
 		if (next > end || next <= addr)
 			next = end;
 		if (pgd_none(*src_pgd))
-			continue;
+			goto next_pgd;
 		if (pgd_bad(*src_pgd)) {
 			pgd_ERROR(*src_pgd);
 			pgd_clear(src_pgd);
-			continue;
+			goto next_pgd;

mm/memory.c int copy_page_range(struct mm_struct *ds · line 459

Quoted 10 lines
mm/memory.c +1 -0
 		}
 		err = copy_pud_range(dst, src, dst_pgd, src_pgd,
 							vma, addr, next);
 		if (err)
 			break;
 
+next_pgd:
 		src_pgd++;
 		dst_pgd++;
 		addr = next;

This looks right to me. Andrew and/or Linus, did this get picked up?