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

[Andrey Konovalov]

On Tue, Jul 14, 2026 at 10:14 AM Xuewen Wang <wangxuewen@xxxxxxxxxx> wrote:
>
> 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 <wangxuewen@xxxxxxxxxx>
> ---
>  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
>

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.