Skip to content

Commit 57efa1f

Browse files
jgunthorpetorvalds
authored andcommitted
mm/gup: prevent gup_fast from racing with COW during fork
Since commit 70e806e ("mm: Do early cow for pinned pages during fork() for ptes") pages under a FOLL_PIN will not be write protected during COW for fork. This means that pages returned from pin_user_pages(FOLL_WRITE) should not become write protected while the pin is active. However, there is a small race where get_user_pages_fast(FOLL_PIN) can establish a FOLL_PIN at the same time copy_present_page() is write protecting it: CPU 0 CPU 1 get_user_pages_fast() internal_get_user_pages_fast() copy_page_range() pte_alloc_map_lock() copy_present_page() atomic_read(has_pinned) == 0 page_maybe_dma_pinned() == false atomic_set(has_pinned, 1); gup_pgd_range() gup_pte_range() pte_t pte = gup_get_pte(ptep) pte_access_permitted(pte) try_grab_compound_head() pte = pte_wrprotect(pte) set_pte_at(); pte_unmap_unlock() // GUP now returns with a write protected page The first attempt to resolve this by using the write protect caused problems (and was missing a barrrier), see commit f3c64ed ("mm: avoid early COW write protect games during fork()") Instead wrap copy_p4d_range() with the write side of a seqcount and check the read side around gup_pgd_range(). If there is a collision then get_user_pages_fast() fails and falls back to slow GUP. Slow GUP is safe against this race because copy_page_range() is only called while holding the exclusive side of the mmap_lock on the src mm_struct. [akpm@linux-foundation.org: coding style fixes] Link: https://lore.kernel.org/r/CAHk-=wi=iCnYCARbPGjkVJu9eyYeZ13N64tZYLdOB8CP5Q_PLw@mail.gmail.com Link: https://lkml.kernel.org/r/2-v4-908497cf359a+4782-gup_fork_jgg@nvidia.com Fixes: f3c64ed ("mm: avoid early COW write protect games during fork()") Signed-off-by: Jason Gunthorpe <jgg@nvidia.com> Suggested-by: Linus Torvalds <torvalds@linux-foundation.org> Reviewed-by: John Hubbard <jhubbard@nvidia.com> Reviewed-by: Jan Kara <jack@suse.cz> Reviewed-by: Peter Xu <peterx@redhat.com> Acked-by: "Ahmed S. Darwish" <a.darwish@linutronix.de> [seqcount_t parts] Cc: Andrea Arcangeli <aarcange@redhat.com> Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> Cc: Christoph Hellwig <hch@lst.de> Cc: Hugh Dickins <hughd@google.com> Cc: Jann Horn <jannh@google.com> Cc: Kirill Shutemov <kirill@shutemov.name> Cc: Kirill Tkhai <ktkhai@virtuozzo.com> Cc: Leon Romanovsky <leonro@nvidia.com> Cc: Michal Hocko <mhocko@suse.com> Cc: Oleg Nesterov <oleg@redhat.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
1 parent c28b1fc commit 57efa1f

File tree

7 files changed

+42
-1
lines changed

7 files changed

+42
-1
lines changed

arch/x86/kernel/tboot.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ static struct mm_struct tboot_mm = {
9393
.pgd = swapper_pg_dir,
9494
.mm_users = ATOMIC_INIT(2),
9595
.mm_count = ATOMIC_INIT(1),
96+
.write_protect_seq = SEQCNT_ZERO(tboot_mm.write_protect_seq),
9697
MMAP_LOCK_INITIALIZER(init_mm)
9798
.page_table_lock = __SPIN_LOCK_UNLOCKED(init_mm.page_table_lock),
9899
.mmlist = LIST_HEAD_INIT(init_mm.mmlist),

drivers/firmware/efi/efi.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ struct mm_struct efi_mm = {
5757
.mm_rb = RB_ROOT,
5858
.mm_users = ATOMIC_INIT(2),
5959
.mm_count = ATOMIC_INIT(1),
60+
.write_protect_seq = SEQCNT_ZERO(efi_mm.write_protect_seq),
6061
MMAP_LOCK_INITIALIZER(efi_mm)
6162
.page_table_lock = __SPIN_LOCK_UNLOCKED(efi_mm.page_table_lock),
6263
.mmlist = LIST_HEAD_INIT(efi_mm.mmlist),

include/linux/mm_types.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
#include <linux/uprobes.h>
1515
#include <linux/page-flags-layout.h>
1616
#include <linux/workqueue.h>
17+
#include <linux/seqlock.h>
1718

1819
#include <asm/mmu.h>
1920

@@ -446,6 +447,13 @@ struct mm_struct {
446447
*/
447448
atomic_t has_pinned;
448449

450+
/**
451+
* @write_protect_seq: Locked when any thread is write
452+
* protecting pages mapped by this mm to enforce a later COW,
453+
* for instance during page table copying for fork().
454+
*/
455+
seqcount_t write_protect_seq;
456+
449457
#ifdef CONFIG_MMU
450458
atomic_long_t pgtables_bytes; /* PTE page table pages */
451459
#endif

kernel/fork.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1007,6 +1007,7 @@ static struct mm_struct *mm_init(struct mm_struct *mm, struct task_struct *p,
10071007
mm->vmacache_seqnum = 0;
10081008
atomic_set(&mm->mm_users, 1);
10091009
atomic_set(&mm->mm_count, 1);
1010+
seqcount_init(&mm->write_protect_seq);
10101011
mmap_init_lock(mm);
10111012
INIT_LIST_HEAD(&mm->mmlist);
10121013
mm->core_state = NULL;

mm/gup.c

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2684,11 +2684,18 @@ static unsigned long lockless_pages_from_mm(unsigned long start,
26842684
{
26852685
unsigned long flags;
26862686
int nr_pinned = 0;
2687+
unsigned seq;
26872688

26882689
if (!IS_ENABLED(CONFIG_HAVE_FAST_GUP) ||
26892690
!gup_fast_permitted(start, end))
26902691
return 0;
26912692

2693+
if (gup_flags & FOLL_PIN) {
2694+
seq = raw_read_seqcount(&current->mm->write_protect_seq);
2695+
if (seq & 1)
2696+
return 0;
2697+
}
2698+
26922699
/*
26932700
* Disable interrupts. The nested form is used, in order to allow full,
26942701
* general purpose use of this routine.
@@ -2703,6 +2710,17 @@ static unsigned long lockless_pages_from_mm(unsigned long start,
27032710
local_irq_save(flags);
27042711
gup_pgd_range(start, end, gup_flags, pages, &nr_pinned);
27052712
local_irq_restore(flags);
2713+
2714+
/*
2715+
* When pinning pages for DMA there could be a concurrent write protect
2716+
* from fork() via copy_page_range(), in this case always fail fast GUP.
2717+
*/
2718+
if (gup_flags & FOLL_PIN) {
2719+
if (read_seqcount_retry(&current->mm->write_protect_seq, seq)) {
2720+
unpin_user_pages(pages, nr_pinned);
2721+
return 0;
2722+
}
2723+
}
27062724
return nr_pinned;
27072725
}
27082726

mm/init-mm.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ struct mm_struct init_mm = {
3131
.pgd = swapper_pg_dir,
3232
.mm_users = ATOMIC_INIT(2),
3333
.mm_count = ATOMIC_INIT(1),
34+
.write_protect_seq = SEQCNT_ZERO(init_mm.write_protect_seq),
3435
MMAP_LOCK_INITIALIZER(init_mm)
3536
.page_table_lock = __SPIN_LOCK_UNLOCKED(init_mm.page_table_lock),
3637
.arg_lock = __SPIN_LOCK_UNLOCKED(init_mm.arg_lock),

mm/memory.c

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1171,6 +1171,15 @@ copy_page_range(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma)
11711171
mmu_notifier_range_init(&range, MMU_NOTIFY_PROTECTION_PAGE,
11721172
0, src_vma, src_mm, addr, end);
11731173
mmu_notifier_invalidate_range_start(&range);
1174+
/*
1175+
* Disabling preemption is not needed for the write side, as
1176+
* the read side doesn't spin, but goes to the mmap_lock.
1177+
*
1178+
* Use the raw variant of the seqcount_t write API to avoid
1179+
* lockdep complaining about preemptibility.
1180+
*/
1181+
mmap_assert_write_locked(src_mm);
1182+
raw_write_seqcount_begin(&src_mm->write_protect_seq);
11741183
}
11751184

11761185
ret = 0;
@@ -1187,8 +1196,10 @@ copy_page_range(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma)
11871196
}
11881197
} while (dst_pgd++, src_pgd++, addr = next, addr != end);
11891198

1190-
if (is_cow)
1199+
if (is_cow) {
1200+
raw_write_seqcount_end(&src_mm->write_protect_seq);
11911201
mmu_notifier_invalidate_range_end(&range);
1202+
}
11921203
return ret;
11931204
}
11941205

0 commit comments

Comments
 (0)