Skip to contentSkip to conversation

Audit 2.6 set_pte users

PATCH Anton Blanchard 2 participants 3 messages Jan 24, 2004

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

Hi,

I went through all the users of set_pte to check if they flush the current pte if it is present. Below is a summary of the audit, everything looks good except for a failure case in dup_mmap->copy_page_range.

The set_pte usage in copy_page_range is fine (no present ptes in the childs address space yet) however ptep_set_wrprotect is also called to write protect COW mappings in the parent. It turns out we can fail part way through copy_page_range and end up not calling flush_tlb_mm. Sure there will be no child process so the COW mappings arent needed, but having the linux view and the hw view of the pte get out of sync is not good.

Anton

--

dup_mmap has problem where we fail calling copy_page (which potentially write protects pages in the parent) and do not call flush_tlb_mm.

===== kernel/fork.c 1.154 vs edited =====

kernel/fork.c +1 -0
@@ -347,6 +347,7 @@
 fail_nomem:
 	retval = -ENOMEM;
 fail:
+	flush_tlb_mm(current->mm);
 	vm_unacct_memory(charge);
 	goto out;
 }

--

fs/exec.c:
put_dirty_page
safe - no existing pte

mm/fremap.c:
install_page
safe - calls ptep_clear_flush
install_file_pte
safe - calls ptep_clear_flush

mm/highmem.c:
map_new_virtual
safe - no existing pte

mm/memory.c:
copy_page_range
safe - no existing pte
zeromap_pte_range
safe - calls flush_tlb_range
remap_pte_range
safe - calls flush_tlb_range
do_swap_page
safe - no existing pte
do_anonymous_page
safe - no existing pte
do_no_page
safe - no existing pte

mm/mprotect.c
change_pte_range
safe - calls flush_tlb_range

mm/mremap.c
copy_one-pte
safe - calls ptep_clear_flush

mm/rmap.c
try_to_unmap_one
safe - calls ptep_clear_flush

mm/swapfile.c:
unuse_pte
safe - no existing pte

mm/vmalloc.c:
map_area_pte
safe - no existing pte

#1 lore raw
Quoted 8 lines · Anton Blanchard

Hi,

I went through all the users of set_pte to check if they flush the current pte if it is present. Below is a summary of the audit, everything looks good except for a failure case in dup_mmap->copy_page_range.

I was hoping this might fix the "missing TLB flush" which Martin Schwidefsky believes is there, and which is causing him grief.

kernel/fork.c line 353

Quoted 10 lines
kernel/fork.c +1 -0
@@ -347,6 +347,7 @@
 fail_nomem:
 	retval = -ENOMEM;
 fail:
+	flush_tlb_mm(current->mm);
 	vm_unacct_memory(charge);
 	goto out;
 }

But look:

retval = 0;
build_mmap_rb(mm);

out:
flush_tlb_mm(current->mm);
up_write(&oldmm->mmap_sem);
return retval;
fail_nomem:
retval = -ENOMEM;
fail:
+ flush_tlb_mm(current->mm);
vm_unacct_memory(charge);
goto out;
}

There is no missing flush here.

#2 lore raw
Quoted 3 lines

I was hoping this might fix the "missing TLB flush" which Martin Schwidefsky believes is there, and which is causing him grief.

...

Quoted 1 line

There is no missing flush here.

Oops, I only parsed one goto deep. Nice how it jumps all over the damn place.

Anton