The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: Eli Billauer <eli.billauer@gmail.com>
To: gregkh@linuxfoundation.org
Cc: arnd@arndb.de, linux-kernel@vger.kernel.org,
	Eli Billauer <eli.billauer@gmail.com>
Subject: [PATCH 6/7] char: xillybus: Add defensive sanity checks
Date: Tue, 30 Jun 2026 11:23:29 +0200	[thread overview]
Message-ID: <20260630092330.43534-7-eli.billauer@gmail.com> (raw)
In-Reply-To: <20260630092330.43534-1-eli.billauer@gmail.com>

Add validation checks for values derived from hardware or user input to
prevent incorrect behavior with malformed data.

Signed-off-by: Eli Billauer <eli.billauer@gmail.com>
---
 drivers/char/xillybus/xillybus_class.c | 11 ++++++++
 drivers/char/xillybus/xillybus_class.h |  4 +++
 drivers/char/xillybus/xillybus_core.c  | 37 ++++++++++++++++++++++++--
 drivers/char/xillybus/xillybus_of.c    |  6 +++++
 drivers/char/xillybus/xillyusb.c       | 29 +++++++++++++++++++-
 5 files changed, 84 insertions(+), 3 deletions(-)

diff --git a/drivers/char/xillybus/xillybus_class.c b/drivers/char/xillybus/xillybus_class.c
index 5e8f03b77064..1eecb8ba244a 100644
--- a/drivers/char/xillybus/xillybus_class.c
+++ b/drivers/char/xillybus/xillybus_class.c
@@ -57,6 +57,9 @@ int xillybus_init_chrdev(struct device *dev,
 	size_t namelen;
 	struct xilly_unit *unit, *u;
 
+	if (num_nodes <= 0 || num_nodes > XILLYBUS_MAX_NODES || !idt || !prefix || !dev)
+		return -ENODEV;
+
 	unit = kzalloc_obj(*unit);
 
 	if (!unit)
@@ -68,6 +71,11 @@ int xillybus_init_chrdev(struct device *dev,
 		snprintf(unit->name, UNITNAMELEN, "%s", prefix);
 
 	for (i = 0; enumerate; i++) {
+		if (i > 99) {
+			dev_err(dev, "Failed to obtain unique unit name\n");
+			goto fail_obtain;
+		}
+
 		snprintf(unit->name, UNITNAMELEN, "%s_%02d",
 			 prefix, i);
 
@@ -219,6 +227,9 @@ int xillybus_find_inode(struct inode *inode,
 	int major = imajor(inode);
 	struct xilly_unit *unit = NULL, *iter;
 
+	if (!inode || !private_data || !index)
+		return -ENODEV;
+
 	mutex_lock(&unit_mutex);
 
 	list_for_each_entry(iter, &unit_list, list_entry)
diff --git a/drivers/char/xillybus/xillybus_class.h b/drivers/char/xillybus/xillybus_class.h
index 5dbfdfc95c65..264b9f6d6793 100644
--- a/drivers/char/xillybus/xillybus_class.h
+++ b/drivers/char/xillybus/xillybus_class.h
@@ -8,6 +8,10 @@
 #ifndef __XILLYBUS_CLASS_H
 #define __XILLYBUS_CLASS_H
 
+#define XILLYBUS_MAX_COUNT (((unsigned int) ~0x1ffff) >> 1)
+#define XILLYBUS_MAX_NODES 1024
+#define XILLYBUS_MAX_IDT 1048576
+
 #include <linux/types.h>
 #include <linux/device.h>
 #include <linux/fs.h>
diff --git a/drivers/char/xillybus/xillybus_core.c b/drivers/char/xillybus/xillybus_core.c
index b264578b2572..3436f16092e0 100644
--- a/drivers/char/xillybus/xillybus_core.c
+++ b/drivers/char/xillybus/xillybus_core.c
@@ -351,6 +351,12 @@ static int xilly_get_dma_buffers(struct xilly_endpoint *ep,
 	struct device *dev = ep->dev;
 	struct xilly_buffer *this_buffer = NULL; /* Init to silence warning */
 
+	if (bytebufsize == 0 || bytebufsize > 0x40000000) {
+		dev_err(ep->dev,
+			"Illegal buffer size requested in IDT. Aborting.\n");
+		return -ENODEV;
+	}
+
 	if (buffers) { /* Not the message buffer */
 		this_buffer = devm_kcalloc(dev, bufnum,
 					   sizeof(struct xilly_buffer),
@@ -623,6 +629,12 @@ static int xilly_scan_idt(struct xilly_endpoint *endpoint,
 		return -ENODEV;
 	}
 
+	if (count == 0 || count > XILLYBUS_MAX_NODES) {
+		dev_err(endpoint->dev,
+			"Unreasonable number of channels. Aborting.\n");
+		return -ENODEV;
+	}
+
 	idt_handle->entries = len >> 2;
 	endpoint->num_channels = count;
 
@@ -707,6 +719,9 @@ static ssize_t xillybus_read(struct file *filp, char __user *userbuf,
 	if (channel->endpoint->fatal_error)
 		return -EIO;
 
+	if (count > XILLYBUS_MAX_COUNT)
+		count = XILLYBUS_MAX_COUNT;
+
 	deadline = jiffies + 1 + XILLY_RX_TIMEOUT;
 
 	rc = mutex_lock_interruptible(&channel->wr_mutex);
@@ -725,8 +740,18 @@ static ssize_t xillybus_read(struct file *filp, char __user *userbuf,
 			bufidx = channel->wr_host_buf_idx;
 			bufpos = channel->wr_host_buf_pos;
 			howmany = ((channel->wr_buffers[bufidx]->end_offset
-				    + 1) << channel->log2_element_size)
-				- bufpos;
+				    + 1) << channel->log2_element_size);
+
+			if (howmany > channel->wr_buf_size ||
+			    howmany < bufpos) {
+				dev_err(channel->endpoint->dev,
+					"Illegal buffer fill level from hardware\n");
+				channel->endpoint->fatal_error = 1;
+				spin_unlock_irqrestore(&channel->wr_spinlock, flags);
+				break;
+			}
+
+			howmany	-= bufpos;
 
 			/* Update wr_host_* to its post-operation state */
 			if (howmany > bytes_to_do) {
@@ -1216,6 +1241,9 @@ static ssize_t xillybus_write(struct file *filp, const char __user *userbuf,
 	if (channel->endpoint->fatal_error)
 		return -EIO;
 
+	if (count > XILLYBUS_MAX_COUNT)
+		count = XILLYBUS_MAX_COUNT;
+
 	rc = mutex_lock_interruptible(&channel->rd_mutex);
 	if (rc)
 		return rc;
@@ -1902,6 +1930,11 @@ int xillybus_endpoint_discovery(struct xilly_endpoint *endpoint)
 		return -ENODEV;
 	}
 
+	if (endpoint->idtlen < 4 || endpoint->idtlen > XILLYBUS_MAX_IDT) {
+		dev_err(endpoint->dev, "Invalid IDT length. Aborting.\n");
+		return -ENODEV;
+	}
+
 	/* Enable DMA */
 	iowrite32((u32) (0x0002 | (endpoint->dma_using_dac & 0x0001)),
 		  endpoint->registers + fpga_dma_control_reg);
diff --git a/drivers/char/xillybus/xillybus_of.c b/drivers/char/xillybus/xillybus_of.c
index 1a1e64133315..22dcc387a5e4 100644
--- a/drivers/char/xillybus/xillybus_of.c
+++ b/drivers/char/xillybus/xillybus_of.c
@@ -53,6 +53,12 @@ static int xilly_drv_probe(struct platform_device *op)
 
 	irq = platform_get_irq(op, 0);
 
+	if (irq < 0) {
+		dev_err(endpoint->dev,
+			"Failed to obtain IRQ number. Aborting.\n");
+		return irq;
+	}
+
 	rc = devm_request_irq(dev, irq, xillybus_isr, 0, xillyname, endpoint);
 
 	if (rc) {
diff --git a/drivers/char/xillybus/xillyusb.c b/drivers/char/xillybus/xillyusb.c
index aa08206a18ef..7459ec9295af 100644
--- a/drivers/char/xillybus/xillyusb.c
+++ b/drivers/char/xillybus/xillyusb.c
@@ -396,6 +396,10 @@ static int fifo_init(struct xillyfifo *fifo,
 	fifo->size = fifo->bufnum * fifo->bufsize;
 	fifo->buf_order = buf_order;
 
+	if (!fifo->size || /* Unsigned integer overflow */
+	    fifo->size > 0x40000000) /* Stay clear from signed int issues */
+		return -ENOMEM; /* Reported as greed for memory */
+
 	fifo->mem = kmalloc_array(fifo->bufnum, sizeof(void *), GFP_KERNEL);
 
 	if (!fifo->mem)
@@ -888,6 +892,7 @@ static int process_in_opcode(struct xillyusb_dev *xdev,
 	struct xillyusb_channel *chan;
 	struct device *dev = xdev->dev;
 	int chan_idx = chan_num >> 1;
+	struct xillyfifo *in_fifo;
 
 	if (chan_idx >= xdev->num_channels) {
 		dev_err(dev, "Received illegal channel ID %d from FPGA\n",
@@ -912,7 +917,10 @@ static int process_in_opcode(struct xillyusb_dev *xdev,
 		 */
 		smp_wmb();
 		WRITE_ONCE(chan->read_data_ok, 0);
-		wake_up_interruptible(&chan->in_fifo->waitq);
+
+		in_fifo = READ_ONCE(chan->in_fifo);
+		if (in_fifo)
+			wake_up_interruptible(&in_fifo->waitq);
 		break;
 
 	case OPCODE_REACHED_CHECKPOINT:
@@ -1443,6 +1451,9 @@ static ssize_t xillyusb_read(struct file *filp, char __user *userbuf,
 	bool sent_set_push = false;
 	int rc;
 
+	if (count > XILLYBUS_MAX_COUNT)
+		count = XILLYBUS_MAX_COUNT;
+
 	deadline = jiffies + 1 + XILLY_RX_TIMEOUT;
 
 	rc = mutex_lock_interruptible(&chan->in_mutex);
@@ -1649,6 +1660,9 @@ static ssize_t xillyusb_write(struct file *filp, const char __user *userbuf,
 	struct xillyfifo *fifo = &chan->out_ep->fifo;
 	int rc;
 
+	if (count > XILLYBUS_MAX_COUNT)
+		count = XILLYBUS_MAX_COUNT;
+
 	rc = mutex_lock_interruptible(&chan->out_mutex);
 
 	if (rc)
@@ -2072,6 +2086,13 @@ static int xillyusb_discovery(struct usb_interface *interface)
 	}
 
 	idt_len = READ_ONCE(idt_fifo.fill);
+
+	if (idt_len < 4 || idt_len > XILLYBUS_MAX_IDT) {
+		rc = -ENODEV;
+		dev_err(&interface->dev, "Invalid IDT length. Aborting.\n");
+		goto unfifo;
+	}
+
 	idt = kmalloc(idt_len, GFP_KERNEL);
 
 	if (!idt) {
@@ -2106,6 +2127,12 @@ static int xillyusb_discovery(struct usb_interface *interface)
 		goto unidt;
 	}
 
+	if (num_channels == 0 || num_channels > XILLYBUS_MAX_NODES) {
+		dev_err(&interface->dev, "Unreasonable number of channels. Aborting.\n");
+		rc = -ENODEV;
+		goto unidt;
+	}
+
 	rc = setup_channels(xdev, (void *)idt + 3, num_channels);
 
 	if (rc)
-- 
2.34.1


  parent reply	other threads:[~2026-06-30  9:24 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-30  9:23 [PATCH 0/7] char: xillybus: Harden driver and improve code quality Eli Billauer
2026-06-30  9:23 ` [PATCH 1/7] char: xillybus: Improve control of execution flow with mutexes Eli Billauer
2026-06-30  9:23 ` [PATCH 2/7] char: xillybus: Remove duplicate error path code Eli Billauer
2026-06-30  9:23 ` [PATCH 3/7] char: xillybus: Avoid possible bandwidth inefficiency Eli Billauer
2026-06-30  9:23 ` [PATCH 4/7] char: xillybus: Use unsigned arithmetic for jiffies differences Eli Billauer
2026-06-30  9:23 ` [PATCH 5/7] char: xillybus: Integer arithmetic improvements Eli Billauer
2026-06-30  9:23 ` Eli Billauer [this message]
2026-06-30  9:23 ` [PATCH 7/7] char: xillybus: Ignore and report unsolicited interrupts Eli Billauer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260630092330.43534-7-eli.billauer@gmail.com \
    --to=eli.billauer@gmail.com \
    --cc=arnd@arndb.de \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox