[PATCH v1] mm/kasan: fix swapped read/write lock usage in stack ring

[Xuewen Wang]

The read-write lock semantics for stack_ring.lock are inverted:

- save_stack_info() modifies stack ring entries but uses read_lock,
which does not protect against concurrent writers and does not
block readers from observing partial updates.
- kasan_complete_mode_report_info() only reads the stack ring but
uses write_lock_irqsave(), which prevents concurrent readers from
walking the ring simultaneously.

Swap the lock types to match their actual access patterns:
- save_stack_info(): read_lock -> write_lock
- kasan_complete_mode_report_info(): write_lock -> read_lock

Fixes: 7bc0584e5d2a ("kasan: implement stack ring for tag-based modes")
Signed-off-by: Xuewen Wang <wangx...@kylinos.cn>
---
mm/kasan/report_tags.c | 4 ++--
mm/kasan/tags.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/mm/kasan/report_tags.c b/mm/kasan/report_tags.c
index d15f8f580e2c..62576b1470e1 100644
--- a/mm/kasan/report_tags.c
+++ b/mm/kasan/report_tags.c
@@ -39,7 +39,7 @@ void kasan_complete_mode_report_info(struct kasan_report_info *info)
return;
}

- write_lock_irqsave(&stack_ring.lock, flags);
+ read_lock_irqsave(&stack_ring.lock, flags);

pos = atomic64_read(&stack_ring.pos);

@@ -99,7 +99,7 @@ void kasan_complete_mode_report_info(struct kasan_report_info *info)
}
}

- write_unlock_irqrestore(&stack_ring.lock, flags);
+ read_unlock_irqrestore(&stack_ring.lock, flags);

/* Assign the common bug type if no entries were found. */
if (!info->bug_type)
diff --git a/mm/kasan/tags.c b/mm/kasan/tags.c
index d65d48b85f90..6aa09122f09e 100644
--- a/mm/kasan/tags.c
+++ b/mm/kasan/tags.c
@@ -110,7 +110,7 @@ static void save_stack_info(struct kmem_cache *cache, void *object,
* Prevent save_stack_info() from modifying stack ring
* when kasan_complete_mode_report_info() is walking it.
*/
- read_lock_irqsave(&stack_ring.lock, flags);
+ write_lock_irqsave(&stack_ring.lock, flags);

next:
pos = atomic64_fetch_add(1, &stack_ring.pos);
@@ -131,7 +131,7 @@ static void save_stack_info(struct kmem_cache *cache, void *object,

entry->ptr = object;

- read_unlock_irqrestore(&stack_ring.lock, flags);
+ write_unlock_irqrestore(&stack_ring.lock, flags);

if (old_stack)
stack_depot_put(old_stack);
--
2.25.1

[Andrey Konovalov]

This is no good.

RW lock here is used to prevent kasan_complete_mode_report_info() from
running concurrently with save_stack_info(). But multiple
save_stack_info() must be allowed to execute concurrently without
contending for a lock. Concurrent writes coming from multiple
save_stack_info() executing at the same time are handled via
atomic64_fetch_add/try_cmpxchg() slot reservation.

[Xuewen Wang]

Hi Andrey,
Thank you for your clarification and correction.
I misunderstood the design intention of this rwlock before. I only judged it from conventional read-write lock semantics, and failed to notice that concurrent writes are guaranteed by slot-level atomic operations and cmpxchg reservation. The lock is specifically used to serialize the report path against all writers, while allowing multiple save_stack_info() instances to run in parallel without lock contention.
This patch is not appropriate, so I will drop it and not proceed with a new version.
Thanks again for your guidance.
Best regards,
Xuewen Wang