lkml.org 
[lkml]   [2019]   [Jun]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
Subject[PATCH 5.1 23/70] habanalabs: fix debugfs code
Date
From: Jann Horn <jannh@google.com>

commit 8438846cce61e284a22316c13aa4b63772963070 upstream.

This fixes multiple things in the habanalabs debugfs code, in particular:

- mmu_write() was unnecessarily verbose, copying around between multiple
buffers
- mmu_write() could write a user-specified, unbounded amount of userspace
memory into a kernel buffer (out-of-bounds write)
- multiple debugfs read handlers ignored the user-supplied count,
potentially corrupting out-of-bounds userspace data
- hl_device_read() was unnecessarily verbose
- hl_device_write() could read uninitialized stack memory
- multiple debugfs read handlers copied terminating null characters to
userspace

Signed-off-by: Jann Horn <jannh@google.com>
Reviewed-by: Oded Gabbay <oded.gabbay@gmail.com>
Signed-off-by: Oded Gabbay <oded.gabbay@gmail.com>
Cc: stable@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
drivers/misc/habanalabs/debugfs.c | 60 +++++++++++---------------------------
1 file changed, 18 insertions(+), 42 deletions(-)

--- a/drivers/misc/habanalabs/debugfs.c
+++ b/drivers/misc/habanalabs/debugfs.c
@@ -459,41 +459,31 @@ static ssize_t mmu_write(struct file *fi
struct hl_debugfs_entry *entry = s->private;
struct hl_dbg_device_entry *dev_entry = entry->dev_entry;
struct hl_device *hdev = dev_entry->hdev;
- char kbuf[MMU_KBUF_SIZE], asid_kbuf[MMU_ASID_BUF_SIZE],
- addr_kbuf[MMU_ADDR_BUF_SIZE];
+ char kbuf[MMU_KBUF_SIZE];
char *c;
ssize_t rc;

if (!hdev->mmu_enable)
return count;

- memset(kbuf, 0, sizeof(kbuf));
- memset(asid_kbuf, 0, sizeof(asid_kbuf));
- memset(addr_kbuf, 0, sizeof(addr_kbuf));
-
+ if (count > sizeof(kbuf) - 1)
+ goto err;
if (copy_from_user(kbuf, buf, count))
goto err;
-
- kbuf[MMU_KBUF_SIZE - 1] = 0;
+ kbuf[count] = 0;

c = strchr(kbuf, ' ');
if (!c)
goto err;
+ *c = '\0';

- memcpy(asid_kbuf, kbuf, c - kbuf);
-
- rc = kstrtouint(asid_kbuf, 10, &dev_entry->mmu_asid);
+ rc = kstrtouint(kbuf, 10, &dev_entry->mmu_asid);
if (rc)
goto err;

- c = strstr(kbuf, " 0x");
- if (!c)
+ if (strncmp(c+1, "0x", 2))
goto err;
-
- c += 3;
- memcpy(addr_kbuf, c, (kbuf + count) - c);
-
- rc = kstrtoull(addr_kbuf, 16, &dev_entry->mmu_addr);
+ rc = kstrtoull(c+3, 16, &dev_entry->mmu_addr);
if (rc)
goto err;

@@ -525,10 +515,8 @@ static ssize_t hl_data_read32(struct fil
}

sprintf(tmp_buf, "0x%08x\n", val);
- rc = simple_read_from_buffer(buf, strlen(tmp_buf) + 1, ppos, tmp_buf,
- strlen(tmp_buf) + 1);
-
- return rc;
+ return simple_read_from_buffer(buf, count, ppos, tmp_buf,
+ strlen(tmp_buf));
}

static ssize_t hl_data_write32(struct file *f, const char __user *buf,
@@ -559,7 +547,6 @@ static ssize_t hl_get_power_state(struct
struct hl_dbg_device_entry *entry = file_inode(f)->i_private;
struct hl_device *hdev = entry->hdev;
char tmp_buf[200];
- ssize_t rc;
int i;

if (*ppos)
@@ -574,10 +561,8 @@ static ssize_t hl_get_power_state(struct

sprintf(tmp_buf,
"current power state: %d\n1 - D0\n2 - D3hot\n3 - Unknown\n", i);
- rc = simple_read_from_buffer(buf, strlen(tmp_buf) + 1, ppos, tmp_buf,
- strlen(tmp_buf) + 1);
-
- return rc;
+ return simple_read_from_buffer(buf, count, ppos, tmp_buf,
+ strlen(tmp_buf));
}

static ssize_t hl_set_power_state(struct file *f, const char __user *buf,
@@ -630,8 +615,8 @@ static ssize_t hl_i2c_data_read(struct f
}

sprintf(tmp_buf, "0x%02x\n", val);
- rc = simple_read_from_buffer(buf, strlen(tmp_buf) + 1, ppos, tmp_buf,
- strlen(tmp_buf) + 1);
+ rc = simple_read_from_buffer(buf, count, ppos, tmp_buf,
+ strlen(tmp_buf));

return rc;
}
@@ -720,18 +705,9 @@ static ssize_t hl_led2_write(struct file
static ssize_t hl_device_read(struct file *f, char __user *buf,
size_t count, loff_t *ppos)
{
- char tmp_buf[200];
- ssize_t rc;
-
- if (*ppos)
- return 0;
-
- sprintf(tmp_buf,
- "Valid values: disable, enable, suspend, resume, cpu_timeout\n");
- rc = simple_read_from_buffer(buf, strlen(tmp_buf) + 1, ppos, tmp_buf,
- strlen(tmp_buf) + 1);
-
- return rc;
+ static const char *help =
+ "Valid values: disable, enable, suspend, resume, cpu_timeout\n";
+ return simple_read_from_buffer(buf, count, ppos, help, strlen(help));
}

static ssize_t hl_device_write(struct file *f, const char __user *buf,
@@ -739,7 +715,7 @@ static ssize_t hl_device_write(struct fi
{
struct hl_dbg_device_entry *entry = file_inode(f)->i_private;
struct hl_device *hdev = entry->hdev;
- char data[30];
+ char data[30] = {0};

/* don't allow partial writes */
if (*ppos != 0)

\
 
 \ /
  Last update: 2019-06-09 19:22    [W:0.374 / U:17.872 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site