On 2024/12/9 16:09, Qi Zheng wrote:
On 2024/12/9 15:56, Yu Zhao wrote:
On Mon, Dec 9, 2024 at 12:00 AM Qi Zheng <zhengqi.arch@xxxxxxxxxxxxx>
wrote:
[...]
If you want syzbot to run the reproducer, reply with:
#syz test: git://repo/address.git branch-or-commit-hash
If you attach or paste a git patch, syzbot will apply it before
testing.
#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git
mm-unstable
diff --git a/mm/memory.c b/mm/memory.c
index 83fd35c034d7a..28526a4205d1b 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -7023,7 +7023,7 @@ static struct kmem_cache *page_ptl_cachep;
void __init ptlock_cache_init(void)
{
page_ptl_cachep = kmem_cache_create("page->ptl",
sizeof(spinlock_t), 0,
- SLAB_PANIC, NULL);
+ SLAB_PANIC|SLAB_TYPESAFE_BY_RCU, NULL);
Note that `SLAB_TYPESAFE_BY_RCU` works by freeing the entire slab (the
page containing the objects) with RCU, not individual objects.
So I don't think this would work. A PTL object can be re-allocated to
someone else, and that new user can re-initialize it. So trying to
concurrently lock it under RCU read lock would also be use-after-free.
Got it. Thanks for pointing this out! So we should put ptlock_free()
into the RCU callback instead of enabling SLAB_TYPESAFE_BY_RCU for
page_ptl_cachep.
Like the following:
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 95bfaf5b85d90..b532415ef5841 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2988,7 +2988,7 @@ void ptlock_free(struct ptdesc *ptdesc);
static inline spinlock_t *ptlock_ptr(struct ptdesc *ptdesc)
{
- return ptdesc->ptl;
+ return &(ptdesc->ptl->ptl);
}
#else /* ALLOC_SPLIT_PTLOCKS */
static inline void ptlock_cache_init(void)
diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index d0e720ccecd71..7b94ea4d0d26a 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -434,6 +434,13 @@ FOLIO_MATCH(flags, _flags_2a);
FOLIO_MATCH(compound_head, _head_2a);
#undef FOLIO_MATCH
+#if ALLOC_SPLIT_PTLOCKS
+struct pt_lock {
+ spinlock_t ptl;
+ struct rcu_head rcu;
+};
+#endif
+
/**
* struct ptdesc - Memory descriptor for page tables.
* @__page_flags: Same as page flags. Powerpc only.
@@ -478,7 +485,7 @@ struct ptdesc {
union {
unsigned long _pt_pad_2;
#if ALLOC_SPLIT_PTLOCKS
- spinlock_t *ptl;
+ struct pt_lock *ptl;
#else
spinlock_t ptl;
#endif
diff --git a/include/linux/mm_types_task.h b/include/linux/mm_types_task.h
index a82aa80c0ba46..774ef2a128104 100644
--- a/include/linux/mm_types_task.h
+++ b/include/linux/mm_types_task.h
@@ -17,7 +17,8 @@
#include <asm/tlbbatch.h>
#endif
-#define ALLOC_SPLIT_PTLOCKS (SPINLOCK_SIZE > BITS_PER_LONG/8)
+/*#define ALLOC_SPLIT_PTLOCKS (SPINLOCK_SIZE > BITS_PER_LONG/8)*/
+#define ALLOC_SPLIT_PTLOCKS 1
/*
* When updating this, please also update struct resident_page_types[] in
diff --git a/mm/memory.c b/mm/memory.c
index 83fd35c034d7a..802dae0602b32 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -7022,24 +7022,34 @@ static struct kmem_cache *page_ptl_cachep;
void __init ptlock_cache_init(void)
{
- page_ptl_cachep = kmem_cache_create("page->ptl",
sizeof(spinlock_t), 0,
+ page_ptl_cachep = kmem_cache_create("page->ptl", sizeof(struct
pt_lock), 0,
SLAB_PANIC, NULL);
}
bool ptlock_alloc(struct ptdesc *ptdesc)
{
- spinlock_t *ptl;
+ struct pt_lock *pt_lock;
- ptl = kmem_cache_alloc(page_ptl_cachep, GFP_KERNEL);
- if (!ptl)
+ pt_lock = kmem_cache_alloc(page_ptl_cachep, GFP_KERNEL);
+ if (!pt_lock)
return false;
- ptdesc->ptl = ptl;
+ ptdesc->ptl = pt_lock;
return true;
}
+static void ptlock_free_rcu(struct rcu_head *head)
+{
+ struct pt_lock *pt_lock;
+
+ pt_lock = container_of(head, struct pt_lock, rcu);
+ kmem_cache_free(page_ptl_cachep, pt_lock);
+}
+
void ptlock_free(struct ptdesc *ptdesc)
{
- kmem_cache_free(page_ptl_cachep, ptdesc->ptl);
+ struct pt_lock *pt_lock = ptdesc->ptl;
+
+ call_rcu(&pt_lock->rcu, ptlock_free_rcu);
}
#endif
