Re: [PATCH v6] soc: aspeed: lpc-snoop: Fix usercopy overflow in snoop_file_read

[Andrew Jeffery <andrew@codeconstruct.com.au>]

On Fri, 2026-06-12 at 19:07 +0000, Karthikeyan KS wrote:
> put_fifo_with_discard() acts as both producer and consumer on the kfifo:
> it calls kfifo_skip() (advances out) and kfifo_put() (advances in) from
> the IRQ handler without synchronizing with snoop_file_read(), which also
> consumes via kfifo_to_user(). On SMP systems this concurrent access can
> leave (in - out) larger than the ring buffer, so __kfifo_to_user()'s clamp
> to (in - out) is ineffective and kfifo_copy_to_user() can attempt a
> copy_to_user() past the kmalloc-2k backing store:
> 
>   usercopy: Kernel memory exposure attempt detected from SLUB object
>   'kmalloc-2k' (offset 0, size 2049)!
>   kernel BUG at mm/usercopy.c!
>   Call trace:
>    usercopy_abort
>    __check_heap_object
>    __check_object_size
>    kfifo_copy_to_user
>    __kfifo_to_user
>    snoop_file_read
>    vfs_read
> 
> Serialize kfifo access with a per-channel spinlock shared between the
> IRQ handler (producer) and the file reader (consumer).  Annotate @fifo
> with __guarded_by(&lock) and opt the driver into context analysis so the
> compiler enforces that all fifo access holds the lock.
> 
> Fixes: 3772e5da4454 ("drivers/misc: Aspeed LPC snoop output using misc chardev")
> Signed-off-by: Karthikeyan KS <karthiproffesional@gmail.com>
> ---
>  drivers/soc/aspeed/Makefile           |  1 +
>  drivers/soc/aspeed/aspeed-lpc-snoop.c | 38 ++++++++++++++++++---------
>  2 files changed, 27 insertions(+), 12 deletions(-)
> 
> Andrew,
> 
> Thanks for the review.
> 
> Changes since v5:
> - Annotate @fifo with __guarded_by(&lock) instead of a comment
> - Move kfifo_initialized() check inside scoped_guard(spinlock, &chan->lock)
>   in put_fifo_with_discard()
> - Replace spin_lock_init() with scoped_guard(spinlock_init, &channel->lock)
>   around kfifo_alloc() in aspeed_lpc_enable_snoop()
> - Enable CONTEXT_ANALYSIS for this driver in drivers/soc/aspeed/Makefile
> 
> Dropped Cc: stable — the fix uses cleanup.h/context-analysis idioms absent
> from LTS; I'll send adapted backports to stable@ once this is in mainline.
> 
> Tested on ast2600-evb (QEMU): 
> 

Can you describe the specific steps you used to test this under qemu?
I'm interested in reproducing your efforts here.

Andrew