From 8438846cce61e284a22316c13aa4b63772963070 Mon Sep 17 00:00:00 2001 From: Jann Horn Date: Sat, 4 May 2019 15:56:08 +0200 Subject: habanalabs: fix debugfs code 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 Reviewed-by: Oded Gabbay Signed-off-by: Oded Gabbay Cc: stable@vger.kernel.org --- drivers/misc/habanalabs/debugfs.c | 60 ++++++++++++--------------------------- 1 file changed, 18 insertions(+), 42 deletions(-) (limited to 'drivers/misc/habanalabs/debugfs.c') diff --git a/drivers/misc/habanalabs/debugfs.c b/drivers/misc/habanalabs/debugfs.c index a4447699ff4e..0ce5621c1324 100644 --- a/drivers/misc/habanalabs/debugfs.c +++ b/drivers/misc/habanalabs/debugfs.c @@ -459,41 +459,31 @@ static ssize_t mmu_write(struct file *file, const char __user *buf, 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; @@ -600,10 +590,8 @@ static ssize_t hl_data_read32(struct file *f, char __user *buf, } 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, @@ -645,7 +633,6 @@ static ssize_t hl_get_power_state(struct file *f, char __user *buf, 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) @@ -660,10 +647,8 @@ static ssize_t hl_get_power_state(struct file *f, char __user *buf, 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, @@ -716,8 +701,8 @@ static ssize_t hl_i2c_data_read(struct file *f, char __user *buf, } 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; } @@ -806,18 +791,9 @@ static ssize_t hl_led2_write(struct file *f, const char __user *buf, 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, @@ -825,7 +801,7 @@ static ssize_t hl_device_write(struct file *f, const char __user *buf, { 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) -- cgit v1.2.3 From e4c814aa7105a6a8320f64ecc48f13bc089eea88 Mon Sep 17 00:00:00 2001 From: Tomer Tayar Date: Mon, 3 Jun 2019 11:25:04 +0000 Subject: habanalabs: Fix virtual address access via debugfs for 2MB pages The debugfs interface for accessing DRAM virtual addresses currently uses the 12 LSBs of a virtual address as an offset. However, it should use the 20 LSBs in case the device MMU page size is 2MB instead of 4KB. This patch fixes the offset calculation to be based on the page size. Signed-off-by: Tomer Tayar Reviewed-by: Oded Gabbay Signed-off-by: Oded Gabbay --- drivers/misc/habanalabs/debugfs.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'drivers/misc/habanalabs/debugfs.c') diff --git a/drivers/misc/habanalabs/debugfs.c b/drivers/misc/habanalabs/debugfs.c index 0ce5621c1324..ba418aaa404c 100644 --- a/drivers/misc/habanalabs/debugfs.c +++ b/drivers/misc/habanalabs/debugfs.c @@ -500,6 +500,7 @@ static int device_va_to_pa(struct hl_device *hdev, u64 virt_addr, { struct hl_ctx *ctx = hdev->user_ctx; u64 hop_addr, hop_pte_addr, hop_pte; + u64 offset_mask = HOP4_MASK | OFFSET_MASK; int rc = 0; if (!ctx) { @@ -542,12 +543,14 @@ static int device_va_to_pa(struct hl_device *hdev, u64 virt_addr, goto not_mapped; hop_pte_addr = get_hop4_pte_addr(ctx, hop_addr, virt_addr); hop_pte = hdev->asic_funcs->read_pte(hdev, hop_pte_addr); + + offset_mask = OFFSET_MASK; } if (!(hop_pte & PAGE_PRESENT_MASK)) goto not_mapped; - *phys_addr = (hop_pte & PTE_PHYS_ADDR_MASK) | (virt_addr & OFFSET_MASK); + *phys_addr = (hop_pte & ~offset_mask) | (virt_addr & offset_mask); goto out; -- cgit v1.2.3