[PATCH] staging: vc04_services: remove BCM2835_VCHIQ_SUPPORT_MEMDUMP

Dan Carpenter dan.carpenter at oracle.com
Thu Sep 21 00:14:09 PDT 2017


BCM2835_VCHIQ_SUPPORT_MEMDUMP lets you look through any user memory.
That's too big of an information leak from a security perspective.  The
debugging dumps need to be more specific to this driver.

Signed-off-by: Dan Carpenter <dan.carpenter at oracle.com>
---
It's possible I have misunderstood.  Untested.

diff --git a/drivers/staging/vc04_services/Kconfig b/drivers/staging/vc04_services/Kconfig
index 9e2763663ab8..f5aaf7d629f0 100644
--- a/drivers/staging/vc04_services/Kconfig
+++ b/drivers/staging/vc04_services/Kconfig
@@ -19,18 +19,6 @@ config BCM2835_VCHIQ
 		Defaults to Y when the Broadcom Videocore services
 		are included in the build, N otherwise.
 
-if BCM2835_VCHIQ
-
-config BCM2835_VCHIQ_SUPPORT_MEMDUMP
-	bool "Support dumping memory contents to debug log"
-	help
-		BCM2835 VCHIQ supports the ability to dump the
-		contents of memory to the debug log.  This
-		is typically only needed by diagnostic tools used
-		to debug issues with VideoCore.
-
-endif
-
 source "drivers/staging/vc04_services/bcm2835-audio/Kconfig"
 
 source "drivers/staging/vc04_services/bcm2835-camera/Kconfig"
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index 314ffac50bb8..d23152bb1379 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -195,11 +195,6 @@ static const char *const ioctl_names[] = {
 vchiq_static_assert(ARRAY_SIZE(ioctl_names) ==
 		    (VCHIQ_IOC_MAX + 1));
 
-#if defined(CONFIG_BCM2835_VCHIQ_SUPPORT_MEMDUMP)
-static void
-dump_phys_mem(void *virt_addr, u32 num_bytes);
-#endif
-
 /****************************************************************************
 *
 *   add_completion
@@ -1161,20 +1156,6 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 				args.handle, args.option, args.value);
 	} break;
 
-#if defined(CONFIG_BCM2835_VCHIQ_SUPPORT_MEMDUMP)
-	case VCHIQ_IOC_DUMP_PHYS_MEM: {
-		VCHIQ_DUMP_MEM_T  args;
-
-		if (copy_from_user
-			 (&args, (const void __user *)arg,
-			  sizeof(args)) != 0) {
-			ret = -EFAULT;
-			break;
-		}
-		dump_phys_mem(args.virt_addr, args.num_bytes);
-	} break;
-#endif
-
 	case VCHIQ_IOC_LIB_VERSION: {
 		unsigned int lib_version = (unsigned int)arg;
 
@@ -1654,42 +1635,6 @@ vchiq_compat_ioctl_get_config(struct file *file,
 	return vchiq_ioctl(file, VCHIQ_IOC_GET_CONFIG, (unsigned long)args);
 }
 
-#if defined(CONFIG_BCM2835_VCHIQ_SUPPORT_MEMDUMP)
-
-struct vchiq_dump_mem32 {
-	compat_uptr_t virt_addr;
-	u32 num_bytes;
-};
-
-#define VCHIQ_IOC_DUMP_PHYS_MEM32 \
-	_IOW(VCHIQ_IOC_MAGIC, 15, struct vchiq_dump_mem32)
-
-static long
-vchiq_compat_ioctl_dump_phys_mem(struct file *file,
-				 unsigned int cmd,
-				 unsigned long arg)
-{
-	VCHIQ_DUMP_MEM_T *args;
-	struct vchiq_dump_mem32 args32;
-
-	args = compat_alloc_user_space(sizeof(*args));
-	if (!args)
-		return -EFAULT;
-
-	if (copy_from_user(&args32,
-			   (struct vchiq_dump_mem32 *)arg,
-			   sizeof(args32)))
-		return -EFAULT;
-
-	if (put_user(compat_ptr(args32.virt_addr), &args->virt_addr) ||
-	    put_user(args32.num_bytes, &args->num_bytes))
-		return -EFAULT;
-
-	return vchiq_ioctl(file, VCHIQ_IOC_DUMP_PHYS_MEM, (unsigned long)args);
-}
-
-#endif
-
 static long
 vchiq_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 {
@@ -1707,10 +1652,6 @@ vchiq_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 		return vchiq_compat_ioctl_dequeue_message(file, cmd, arg);
 	case VCHIQ_IOC_GET_CONFIG32:
 		return vchiq_compat_ioctl_get_config(file, cmd, arg);
-#if defined(CONFIG_BCM2835_VCHIQ_SUPPORT_MEMDUMP)
-	case VCHIQ_IOC_DUMP_PHYS_MEM32:
-		return vchiq_compat_ioctl_dump_phys_mem(file, cmd, arg);
-#endif
 	default:
 		return vchiq_ioctl(file, cmd, arg);
 	}
@@ -2050,98 +1991,6 @@ vchiq_dump_platform_service_state(void *dump_context, VCHIQ_SERVICE_T *service)
 
 /****************************************************************************
 *
-*   dump_user_mem
-*
-***************************************************************************/
-
-#if defined(CONFIG_BCM2835_VCHIQ_SUPPORT_MEMDUMP)
-
-static void
-dump_phys_mem(void *virt_addr, u32 num_bytes)
-{
-	int            rc;
-	u8            *end_virt_addr = virt_addr + num_bytes;
-	int            num_pages;
-	int            offset;
-	int            end_offset;
-	int            page_idx;
-	int            prev_idx;
-	struct page   *page;
-	struct page  **pages;
-	u8            *kmapped_virt_ptr;
-
-	/* Align virt_addr and end_virt_addr to 16 byte boundaries. */
-
-	virt_addr = (void *)((unsigned long)virt_addr & ~0x0fuL);
-	end_virt_addr = (void *)(((unsigned long)end_virt_addr + 15uL) &
-		~0x0fuL);
-
-	offset = (int)(long)virt_addr & (PAGE_SIZE - 1);
-	end_offset = (int)(long)end_virt_addr & (PAGE_SIZE - 1);
-
-	num_pages = DIV_ROUND_UP(offset + num_bytes, PAGE_SIZE);
-
-	pages = kmalloc(sizeof(struct page *) * num_pages, GFP_KERNEL);
-	if (!pages) {
-		vchiq_log_error(vchiq_arm_log_level,
-			"Unable to allocation memory for %d pages\n",
-			num_pages);
-		return;
-	}
-
-	down_read(&current->mm->mmap_sem);
-	rc = get_user_pages(
-		(unsigned long)virt_addr, /* start */
-		num_pages,                /* len */
-		0,                        /* gup_flags */
-		pages,                    /* pages (array of page pointers) */
-		NULL);                    /* vmas */
-	up_read(&current->mm->mmap_sem);
-
-	prev_idx = -1;
-	page = NULL;
-
-	if (rc < 0) {
-		vchiq_log_error(vchiq_arm_log_level,
-				"Failed to get user pages: %d\n", rc);
-		goto out;
-	}
-
-	while (offset < end_offset) {
-		int page_offset = offset % PAGE_SIZE;
-
-		page_idx = offset / PAGE_SIZE;
-		if (page_idx != prev_idx) {
-			if (page != NULL)
-				kunmap(page);
-			page = pages[page_idx];
-			kmapped_virt_ptr = kmap(page);
-			prev_idx = page_idx;
-		}
-
-		if (vchiq_arm_log_level >= VCHIQ_LOG_TRACE)
-			vchiq_log_dump_mem("ph",
-				(u32)(unsigned long)&kmapped_virt_ptr[
-					page_offset],
-				&kmapped_virt_ptr[page_offset], 16);
-
-		offset += 16;
-	}
-
-out:
-	if (page != NULL)
-		kunmap(page);
-
-	for (page_idx = 0; page_idx < num_pages; page_idx++)
-		put_page(pages[page_idx]);
-
-	kfree(pages);
-}
-
-#endif
-
-/****************************************************************************
-*
 *   vchiq_read
 *
 ***************************************************************************/



More information about the linux-rpi-kernel mailing list