soc: aspeed: lpc-snoop: Fix usercopy overflow in snoop_file_read

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: 3772e5da44 ("drivers/misc: Aspeed LPC snoop output using misc chardev")
Signed-off-by: Karthikeyan KS <karthiproffesional@gmail.com>
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
This commit is contained in:
Karthikeyan KS
2026-06-12 19:07:43 +00:00
committed by Andrew Jeffery
parent dc59e4fea9
commit 1acef6d85b
2 changed files with 27 additions and 12 deletions

View File

@@ -4,3 +4,4 @@ obj-$(CONFIG_ASPEED_LPC_SNOOP) += aspeed-lpc-snoop.o
obj-$(CONFIG_ASPEED_UART_ROUTING) += aspeed-uart-routing.o
obj-$(CONFIG_ASPEED_P2A_CTRL) += aspeed-p2a-ctrl.o
obj-$(CONFIG_ASPEED_SOCINFO) += aspeed-socinfo.o
CONTEXT_ANALYSIS_aspeed-lpc-snoop.o := y

View File

@@ -11,6 +11,7 @@
*/
#include <linux/bitops.h>
#include <linux/cleanup.h>
#include <linux/clk.h>
#include <linux/dev_printk.h>
#include <linux/interrupt.h>
@@ -74,7 +75,8 @@ struct aspeed_lpc_snoop_channel_cfg {
struct aspeed_lpc_snoop_channel {
const struct aspeed_lpc_snoop_channel_cfg *cfg;
bool enabled;
struct kfifo fifo;
spinlock_t lock;
struct kfifo fifo __guarded_by(&lock);
wait_queue_head_t wq;
struct miscdevice miscdev;
};
@@ -114,6 +116,7 @@ static ssize_t snoop_file_read(struct file *file, char __user *buffer,
size_t count, loff_t *ppos)
{
struct aspeed_lpc_snoop_channel *chan = snoop_file_to_chan(file);
u8 *buf __free(kfree) = NULL;
unsigned int copied;
int ret = 0;
@@ -125,9 +128,16 @@ static ssize_t snoop_file_read(struct file *file, char __user *buffer,
if (ret == -ERESTARTSYS)
return -EINTR;
}
ret = kfifo_to_user(&chan->fifo, buffer, count, &copied);
if (ret)
return ret;
count = min_t(size_t, count, SNOOP_FIFO_SIZE);
buf = kmalloc(count, GFP_KERNEL);
if (!buf)
return -ENOMEM;
copied = kfifo_out_spinlocked(&chan->fifo, buf, count, &chan->lock);
if (copied && copy_to_user(buffer, buf, copied))
return -EFAULT;
return copied;
}
@@ -151,11 +161,13 @@ static const struct file_operations snoop_fops = {
/* Save a byte to a FIFO and discard the oldest byte if FIFO is full */
static void put_fifo_with_discard(struct aspeed_lpc_snoop_channel *chan, u8 val)
{
if (!kfifo_initialized(&chan->fifo))
return;
if (kfifo_is_full(&chan->fifo))
kfifo_skip(&chan->fifo);
kfifo_put(&chan->fifo, val);
scoped_guard(spinlock, &chan->lock) {
if (!kfifo_initialized(&chan->fifo))
return;
if (kfifo_is_full(&chan->fifo))
kfifo_skip(&chan->fifo);
kfifo_put(&chan->fifo, val);
}
wake_up_interruptible(&chan->wq);
}
@@ -239,9 +251,11 @@ static int aspeed_lpc_enable_snoop(struct device *dev,
if (!channel->miscdev.name)
return -ENOMEM;
rc = kfifo_alloc(&channel->fifo, SNOOP_FIFO_SIZE, GFP_KERNEL);
if (rc)
return rc;
scoped_guard(spinlock_init, &channel->lock) {
rc = kfifo_alloc(&channel->fifo, SNOOP_FIFO_SIZE, GFP_KERNEL);
if (rc)
return rc;
}
rc = misc_register(&channel->miscdev);
if (rc)