[Trusty,SRU,1/1] mm: Tighten x86 /dev/mem with zeroing reads

Message ID 20171122130731.18779-2-kleber.souza@canonical.com
State New
Headers show
Series
  • Fix for CVE-2017-7889
Related show

Commit Message

Kleber Souza Nov. 22, 2017, 1:07 p.m.
From: Kees Cook <keescook@chromium.org>

commit a4866aa812518ed1a37d8ea0c881dc946409de94 upstream.

Under CONFIG_STRICT_DEVMEM, reading System RAM through /dev/mem is
disallowed. However, on x86, the first 1MB was always allowed for BIOS
and similar things, regardless of it actually being System RAM. It was
possible for heap to end up getting allocated in low 1MB RAM, and then
read by things like x86info or dd, which would trip hardened usercopy:

usercopy: kernel memory exposure attempt detected from ffff880000090000 (dma-kmalloc-256) (4096 bytes)

This changes the x86 exception for the low 1MB by reading back zeros for
System RAM areas instead of blindly allowing them. More work is needed to
extend this to mmap, but currently mmap doesn't go through usercopy, so
hardened usercopy won't Oops the kernel.

Reported-by: Tommi Rantala <tommi.t.rantala@nokia.com>
Tested-by: Tommi Rantala <tommi.t.rantala@nokia.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
[bwh: Backported to 3.16: adjust context]
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>

CVE-2017-7889
(cherry picked from commit 3cbd86d25eeb61e57cb3367fe302c271b0c70fb2 linux-stable)
Signed-off-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com>
---
 arch/x86/mm/init.c | 41 +++++++++++++++++++--------
 drivers/char/mem.c | 82 ++++++++++++++++++++++++++++++++++--------------------
 2 files changed, 82 insertions(+), 41 deletions(-)

Comments

Stefan Bader Dec. 12, 2017, 9:38 a.m. | #1
On 22.11.2017 13:07, Kleber Sacilotto de Souza wrote:
> From: Kees Cook <keescook@chromium.org>
> 
> commit a4866aa812518ed1a37d8ea0c881dc946409de94 upstream.
> 
> Under CONFIG_STRICT_DEVMEM, reading System RAM through /dev/mem is
> disallowed. However, on x86, the first 1MB was always allowed for BIOS
> and similar things, regardless of it actually being System RAM. It was
> possible for heap to end up getting allocated in low 1MB RAM, and then
> read by things like x86info or dd, which would trip hardened usercopy:
> 
> usercopy: kernel memory exposure attempt detected from ffff880000090000 (dma-kmalloc-256) (4096 bytes)
> 
> This changes the x86 exception for the low 1MB by reading back zeros for
> System RAM areas instead of blindly allowing them. More work is needed to
> extend this to mmap, but currently mmap doesn't go through usercopy, so
> hardened usercopy won't Oops the kernel.
> 
> Reported-by: Tommi Rantala <tommi.t.rantala@nokia.com>
> Tested-by: Tommi Rantala <tommi.t.rantala@nokia.com>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> [bwh: Backported to 3.16: adjust context]
> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> 
> CVE-2017-7889
> (cherry picked from commit 3cbd86d25eeb61e57cb3367fe302c271b0c70fb2 linux-stable)
> Signed-off-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com>
Acked-by: Stefan Bader <stefan.bader@canonical.com>

> ---
>  arch/x86/mm/init.c | 41 +++++++++++++++++++--------
>  drivers/char/mem.c | 82 ++++++++++++++++++++++++++++++++++--------------------
>  2 files changed, 82 insertions(+), 41 deletions(-)
> 
> diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
> index f97130618113..89c43a1ce82b 100644
> --- a/arch/x86/mm/init.c
> +++ b/arch/x86/mm/init.c
> @@ -573,21 +573,40 @@ void __init init_mem_mapping(void)
>   * devmem_is_allowed() checks to see if /dev/mem access to a certain address
>   * is valid. The argument is a physical page number.
>   *
> - *
> - * On x86, access has to be given to the first megabyte of ram because that area
> - * contains bios code and data regions used by X and dosemu and similar apps.
> - * Access has to be given to non-kernel-ram areas as well, these contain the PCI
> - * mmio resources as well as potential bios/acpi data regions.
> + * On x86, access has to be given to the first megabyte of RAM because that
> + * area traditionally contains BIOS code and data regions used by X, dosemu,
> + * and similar apps. Since they map the entire memory range, the whole range
> + * must be allowed (for mapping), but any areas that would otherwise be
> + * disallowed are flagged as being "zero filled" instead of rejected.
> + * Access has to be given to non-kernel-ram areas as well, these contain the
> + * PCI mmio resources as well as potential bios/acpi data regions.
>   */
>  int devmem_is_allowed(unsigned long pagenr)
>  {
> -	if (pagenr < 256)
> -		return 1;
> -	if (iomem_is_exclusive(pagenr << PAGE_SHIFT))
> +	if (page_is_ram(pagenr)) {
> +		/*
> +		 * For disallowed memory regions in the low 1MB range,
> +		 * request that the page be shown as all zeros.
> +		 */
> +		if (pagenr < 256)
> +			return 2;
> +
> +		return 0;
> +	}
> +
> +	/*
> +	 * This must follow RAM test, since System RAM is considered a
> +	 * restricted resource under CONFIG_STRICT_IOMEM.
> +	 */
> +	if (iomem_is_exclusive(pagenr << PAGE_SHIFT)) {
> +		/* Low 1MB bypasses iomem restrictions. */
> +		if (pagenr < 256)
> +			return 1;
> +
>  		return 0;
> -	if (!page_is_ram(pagenr))
> -		return 1;
> -	return 0;
> +	}
> +
> +	return 1;
>  }
>  
>  void free_init_pages(char *what, unsigned long begin, unsigned long end)
> diff --git a/drivers/char/mem.c b/drivers/char/mem.c
> index 917403fe10da..5c2b7c575c9d 100644
> --- a/drivers/char/mem.c
> +++ b/drivers/char/mem.c
> @@ -59,6 +59,10 @@ static inline int valid_mmap_phys_addr_range(unsigned long pfn, size_t size)
>  #endif
>  
>  #ifdef CONFIG_STRICT_DEVMEM
> +static inline int page_is_allowed(unsigned long pfn)
> +{
> +	return devmem_is_allowed(pfn);
> +}
>  static inline int range_is_allowed(unsigned long pfn, unsigned long size)
>  {
>  	u64 from = ((u64)pfn) << PAGE_SHIFT;
> @@ -78,6 +82,10 @@ static inline int range_is_allowed(unsigned long pfn, unsigned long size)
>  	return 1;
>  }
>  #else
> +static inline int page_is_allowed(unsigned long pfn)
> +{
> +	return 1;
> +}
>  static inline int range_is_allowed(unsigned long pfn, unsigned long size)
>  {
>  	return 1;
> @@ -122,23 +130,31 @@ static ssize_t read_mem(struct file *file, char __user *buf,
>  
>  	while (count > 0) {
>  		unsigned long remaining;
> +		int allowed;
>  
>  		sz = size_inside_page(p, count);
>  
> -		if (!range_is_allowed(p >> PAGE_SHIFT, count))
> +		allowed = page_is_allowed(p >> PAGE_SHIFT);
> +		if (!allowed)
>  			return -EPERM;
> +		if (allowed == 2) {
> +			/* Show zeros for restricted memory. */
> +			remaining = clear_user(buf, sz);
> +		} else {
> +			/*
> +			 * On ia64 if a page has been mapped somewhere as
> +			 * uncached, then it must also be accessed uncached
> +			 * by the kernel or data corruption may occur.
> +			 */
> +			ptr = xlate_dev_mem_ptr(p);
> +			if (!ptr)
> +				return -EFAULT;
>  
> -		/*
> -		 * On ia64 if a page has been mapped somewhere as uncached, then
> -		 * it must also be accessed uncached by the kernel or data
> -		 * corruption may occur.
> -		 */
> -		ptr = xlate_dev_mem_ptr(p);
> -		if (!ptr)
> -			return -EFAULT;
> +			remaining = copy_to_user(buf, ptr, sz);
> +
> +			unxlate_dev_mem_ptr(p, ptr);
> +		}
>  
> -		remaining = copy_to_user(buf, ptr, sz);
> -		unxlate_dev_mem_ptr(p, ptr);
>  		if (remaining)
>  			return -EFAULT;
>  
> @@ -181,30 +197,36 @@ static ssize_t write_mem(struct file *file, const char __user *buf,
>  #endif
>  
>  	while (count > 0) {
> +		int allowed;
> +
>  		sz = size_inside_page(p, count);
>  
> -		if (!range_is_allowed(p >> PAGE_SHIFT, sz))
> +		allowed = page_is_allowed(p >> PAGE_SHIFT);
> +		if (!allowed)
>  			return -EPERM;
>  
> -		/*
> -		 * On ia64 if a page has been mapped somewhere as uncached, then
> -		 * it must also be accessed uncached by the kernel or data
> -		 * corruption may occur.
> -		 */
> -		ptr = xlate_dev_mem_ptr(p);
> -		if (!ptr) {
> -			if (written)
> -				break;
> -			return -EFAULT;
> -		}
> +		/* Skip actual writing when a page is marked as restricted. */
> +		if (allowed == 1) {
> +			/*
> +			 * On ia64 if a page has been mapped somewhere as
> +			 * uncached, then it must also be accessed uncached
> +			 * by the kernel or data corruption may occur.
> +			 */
> +			ptr = xlate_dev_mem_ptr(p);
> +			if (!ptr) {
> +				if (written)
> +					break;
> +				return -EFAULT;
> +			}
>  
> -		copied = copy_from_user(ptr, buf, sz);
> -		unxlate_dev_mem_ptr(p, ptr);
> -		if (copied) {
> -			written += sz - copied;
> -			if (written)
> -				break;
> -			return -EFAULT;
> +			copied = copy_from_user(ptr, buf, sz);
> +			unxlate_dev_mem_ptr(p, ptr);
> +			if (copied) {
> +				written += sz - copied;
> +				if (written)
> +					break;
> +				return -EFAULT;
> +			}
>  		}
>  
>  		buf += sz;
>
Colin Ian King Dec. 12, 2017, 9:48 a.m. | #2
On 22/11/17 13:07, Kleber Sacilotto de Souza wrote:
> From: Kees Cook <keescook@chromium.org>
> 
> commit a4866aa812518ed1a37d8ea0c881dc946409de94 upstream.
> 
> Under CONFIG_STRICT_DEVMEM, reading System RAM through /dev/mem is
> disallowed. However, on x86, the first 1MB was always allowed for BIOS
> and similar things, regardless of it actually being System RAM. It was
> possible for heap to end up getting allocated in low 1MB RAM, and then
> read by things like x86info or dd, which would trip hardened usercopy:
> 
> usercopy: kernel memory exposure attempt detected from ffff880000090000 (dma-kmalloc-256) (4096 bytes)
> 
> This changes the x86 exception for the low 1MB by reading back zeros for
> System RAM areas instead of blindly allowing them. More work is needed to
> extend this to mmap, but currently mmap doesn't go through usercopy, so
> hardened usercopy won't Oops the kernel.
> 
> Reported-by: Tommi Rantala <tommi.t.rantala@nokia.com>
> Tested-by: Tommi Rantala <tommi.t.rantala@nokia.com>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> [bwh: Backported to 3.16: adjust context]
> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> 
> CVE-2017-7889
> (cherry picked from commit 3cbd86d25eeb61e57cb3367fe302c271b0c70fb2 linux-stable)
> Signed-off-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com>
> ---
>  arch/x86/mm/init.c | 41 +++++++++++++++++++--------
>  drivers/char/mem.c | 82 ++++++++++++++++++++++++++++++++++--------------------
>  2 files changed, 82 insertions(+), 41 deletions(-)
> 
> diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
> index f97130618113..89c43a1ce82b 100644
> --- a/arch/x86/mm/init.c
> +++ b/arch/x86/mm/init.c
> @@ -573,21 +573,40 @@ void __init init_mem_mapping(void)
>   * devmem_is_allowed() checks to see if /dev/mem access to a certain address
>   * is valid. The argument is a physical page number.
>   *
> - *
> - * On x86, access has to be given to the first megabyte of ram because that area
> - * contains bios code and data regions used by X and dosemu and similar apps.
> - * Access has to be given to non-kernel-ram areas as well, these contain the PCI
> - * mmio resources as well as potential bios/acpi data regions.
> + * On x86, access has to be given to the first megabyte of RAM because that
> + * area traditionally contains BIOS code and data regions used by X, dosemu,
> + * and similar apps. Since they map the entire memory range, the whole range
> + * must be allowed (for mapping), but any areas that would otherwise be
> + * disallowed are flagged as being "zero filled" instead of rejected.
> + * Access has to be given to non-kernel-ram areas as well, these contain the
> + * PCI mmio resources as well as potential bios/acpi data regions.
>   */
>  int devmem_is_allowed(unsigned long pagenr)
>  {
> -	if (pagenr < 256)
> -		return 1;
> -	if (iomem_is_exclusive(pagenr << PAGE_SHIFT))
> +	if (page_is_ram(pagenr)) {
> +		/*
> +		 * For disallowed memory regions in the low 1MB range,
> +		 * request that the page be shown as all zeros.
> +		 */
> +		if (pagenr < 256)
> +			return 2;
> +
> +		return 0;
> +	}
> +
> +	/*
> +	 * This must follow RAM test, since System RAM is considered a
> +	 * restricted resource under CONFIG_STRICT_IOMEM.
> +	 */
> +	if (iomem_is_exclusive(pagenr << PAGE_SHIFT)) {
> +		/* Low 1MB bypasses iomem restrictions. */
> +		if (pagenr < 256)
> +			return 1;
> +
>  		return 0;
> -	if (!page_is_ram(pagenr))
> -		return 1;
> -	return 0;
> +	}
> +
> +	return 1;
>  }
>  
>  void free_init_pages(char *what, unsigned long begin, unsigned long end)
> diff --git a/drivers/char/mem.c b/drivers/char/mem.c
> index 917403fe10da..5c2b7c575c9d 100644
> --- a/drivers/char/mem.c
> +++ b/drivers/char/mem.c
> @@ -59,6 +59,10 @@ static inline int valid_mmap_phys_addr_range(unsigned long pfn, size_t size)
>  #endif
>  
>  #ifdef CONFIG_STRICT_DEVMEM
> +static inline int page_is_allowed(unsigned long pfn)
> +{
> +	return devmem_is_allowed(pfn);
> +}
>  static inline int range_is_allowed(unsigned long pfn, unsigned long size)
>  {
>  	u64 from = ((u64)pfn) << PAGE_SHIFT;
> @@ -78,6 +82,10 @@ static inline int range_is_allowed(unsigned long pfn, unsigned long size)
>  	return 1;
>  }
>  #else
> +static inline int page_is_allowed(unsigned long pfn)
> +{
> +	return 1;
> +}
>  static inline int range_is_allowed(unsigned long pfn, unsigned long size)
>  {
>  	return 1;
> @@ -122,23 +130,31 @@ static ssize_t read_mem(struct file *file, char __user *buf,
>  
>  	while (count > 0) {
>  		unsigned long remaining;
> +		int allowed;
>  
>  		sz = size_inside_page(p, count);
>  
> -		if (!range_is_allowed(p >> PAGE_SHIFT, count))
> +		allowed = page_is_allowed(p >> PAGE_SHIFT);
> +		if (!allowed)
>  			return -EPERM;
> +		if (allowed == 2) {
> +			/* Show zeros for restricted memory. */
> +			remaining = clear_user(buf, sz);
> +		} else {
> +			/*
> +			 * On ia64 if a page has been mapped somewhere as
> +			 * uncached, then it must also be accessed uncached
> +			 * by the kernel or data corruption may occur.
> +			 */
> +			ptr = xlate_dev_mem_ptr(p);
> +			if (!ptr)
> +				return -EFAULT;
>  
> -		/*
> -		 * On ia64 if a page has been mapped somewhere as uncached, then
> -		 * it must also be accessed uncached by the kernel or data
> -		 * corruption may occur.
> -		 */
> -		ptr = xlate_dev_mem_ptr(p);
> -		if (!ptr)
> -			return -EFAULT;
> +			remaining = copy_to_user(buf, ptr, sz);
> +
> +			unxlate_dev_mem_ptr(p, ptr);
> +		}
>  
> -		remaining = copy_to_user(buf, ptr, sz);
> -		unxlate_dev_mem_ptr(p, ptr);
>  		if (remaining)
>  			return -EFAULT;
>  
> @@ -181,30 +197,36 @@ static ssize_t write_mem(struct file *file, const char __user *buf,
>  #endif
>  
>  	while (count > 0) {
> +		int allowed;
> +
>  		sz = size_inside_page(p, count);
>  
> -		if (!range_is_allowed(p >> PAGE_SHIFT, sz))
> +		allowed = page_is_allowed(p >> PAGE_SHIFT);
> +		if (!allowed)
>  			return -EPERM;
>  
> -		/*
> -		 * On ia64 if a page has been mapped somewhere as uncached, then
> -		 * it must also be accessed uncached by the kernel or data
> -		 * corruption may occur.
> -		 */
> -		ptr = xlate_dev_mem_ptr(p);
> -		if (!ptr) {
> -			if (written)
> -				break;
> -			return -EFAULT;
> -		}
> +		/* Skip actual writing when a page is marked as restricted. */
> +		if (allowed == 1) {
> +			/*
> +			 * On ia64 if a page has been mapped somewhere as
> +			 * uncached, then it must also be accessed uncached
> +			 * by the kernel or data corruption may occur.
> +			 */
> +			ptr = xlate_dev_mem_ptr(p);
> +			if (!ptr) {
> +				if (written)
> +					break;
> +				return -EFAULT;
> +			}
>  
> -		copied = copy_from_user(ptr, buf, sz);
> -		unxlate_dev_mem_ptr(p, ptr);
> -		if (copied) {
> -			written += sz - copied;
> -			if (written)
> -				break;
> -			return -EFAULT;
> +			copied = copy_from_user(ptr, buf, sz);
> +			unxlate_dev_mem_ptr(p, ptr);
> +			if (copied) {
> +				written += sz - copied;
> +				if (written)
> +					break;
> +				return -EFAULT;
> +			}
>  		}
>  
>  		buf += sz;
>
Acked-by: Colin Ian King <colin.king@canonical.com>

Patch

diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
index f97130618113..89c43a1ce82b 100644
--- a/arch/x86/mm/init.c
+++ b/arch/x86/mm/init.c
@@ -573,21 +573,40 @@  void __init init_mem_mapping(void)
  * devmem_is_allowed() checks to see if /dev/mem access to a certain address
  * is valid. The argument is a physical page number.
  *
- *
- * On x86, access has to be given to the first megabyte of ram because that area
- * contains bios code and data regions used by X and dosemu and similar apps.
- * Access has to be given to non-kernel-ram areas as well, these contain the PCI
- * mmio resources as well as potential bios/acpi data regions.
+ * On x86, access has to be given to the first megabyte of RAM because that
+ * area traditionally contains BIOS code and data regions used by X, dosemu,
+ * and similar apps. Since they map the entire memory range, the whole range
+ * must be allowed (for mapping), but any areas that would otherwise be
+ * disallowed are flagged as being "zero filled" instead of rejected.
+ * Access has to be given to non-kernel-ram areas as well, these contain the
+ * PCI mmio resources as well as potential bios/acpi data regions.
  */
 int devmem_is_allowed(unsigned long pagenr)
 {
-	if (pagenr < 256)
-		return 1;
-	if (iomem_is_exclusive(pagenr << PAGE_SHIFT))
+	if (page_is_ram(pagenr)) {
+		/*
+		 * For disallowed memory regions in the low 1MB range,
+		 * request that the page be shown as all zeros.
+		 */
+		if (pagenr < 256)
+			return 2;
+
+		return 0;
+	}
+
+	/*
+	 * This must follow RAM test, since System RAM is considered a
+	 * restricted resource under CONFIG_STRICT_IOMEM.
+	 */
+	if (iomem_is_exclusive(pagenr << PAGE_SHIFT)) {
+		/* Low 1MB bypasses iomem restrictions. */
+		if (pagenr < 256)
+			return 1;
+
 		return 0;
-	if (!page_is_ram(pagenr))
-		return 1;
-	return 0;
+	}
+
+	return 1;
 }
 
 void free_init_pages(char *what, unsigned long begin, unsigned long end)
diff --git a/drivers/char/mem.c b/drivers/char/mem.c
index 917403fe10da..5c2b7c575c9d 100644
--- a/drivers/char/mem.c
+++ b/drivers/char/mem.c
@@ -59,6 +59,10 @@  static inline int valid_mmap_phys_addr_range(unsigned long pfn, size_t size)
 #endif
 
 #ifdef CONFIG_STRICT_DEVMEM
+static inline int page_is_allowed(unsigned long pfn)
+{
+	return devmem_is_allowed(pfn);
+}
 static inline int range_is_allowed(unsigned long pfn, unsigned long size)
 {
 	u64 from = ((u64)pfn) << PAGE_SHIFT;
@@ -78,6 +82,10 @@  static inline int range_is_allowed(unsigned long pfn, unsigned long size)
 	return 1;
 }
 #else
+static inline int page_is_allowed(unsigned long pfn)
+{
+	return 1;
+}
 static inline int range_is_allowed(unsigned long pfn, unsigned long size)
 {
 	return 1;
@@ -122,23 +130,31 @@  static ssize_t read_mem(struct file *file, char __user *buf,
 
 	while (count > 0) {
 		unsigned long remaining;
+		int allowed;
 
 		sz = size_inside_page(p, count);
 
-		if (!range_is_allowed(p >> PAGE_SHIFT, count))
+		allowed = page_is_allowed(p >> PAGE_SHIFT);
+		if (!allowed)
 			return -EPERM;
+		if (allowed == 2) {
+			/* Show zeros for restricted memory. */
+			remaining = clear_user(buf, sz);
+		} else {
+			/*
+			 * On ia64 if a page has been mapped somewhere as
+			 * uncached, then it must also be accessed uncached
+			 * by the kernel or data corruption may occur.
+			 */
+			ptr = xlate_dev_mem_ptr(p);
+			if (!ptr)
+				return -EFAULT;
 
-		/*
-		 * On ia64 if a page has been mapped somewhere as uncached, then
-		 * it must also be accessed uncached by the kernel or data
-		 * corruption may occur.
-		 */
-		ptr = xlate_dev_mem_ptr(p);
-		if (!ptr)
-			return -EFAULT;
+			remaining = copy_to_user(buf, ptr, sz);
+
+			unxlate_dev_mem_ptr(p, ptr);
+		}
 
-		remaining = copy_to_user(buf, ptr, sz);
-		unxlate_dev_mem_ptr(p, ptr);
 		if (remaining)
 			return -EFAULT;
 
@@ -181,30 +197,36 @@  static ssize_t write_mem(struct file *file, const char __user *buf,
 #endif
 
 	while (count > 0) {
+		int allowed;
+
 		sz = size_inside_page(p, count);
 
-		if (!range_is_allowed(p >> PAGE_SHIFT, sz))
+		allowed = page_is_allowed(p >> PAGE_SHIFT);
+		if (!allowed)
 			return -EPERM;
 
-		/*
-		 * On ia64 if a page has been mapped somewhere as uncached, then
-		 * it must also be accessed uncached by the kernel or data
-		 * corruption may occur.
-		 */
-		ptr = xlate_dev_mem_ptr(p);
-		if (!ptr) {
-			if (written)
-				break;
-			return -EFAULT;
-		}
+		/* Skip actual writing when a page is marked as restricted. */
+		if (allowed == 1) {
+			/*
+			 * On ia64 if a page has been mapped somewhere as
+			 * uncached, then it must also be accessed uncached
+			 * by the kernel or data corruption may occur.
+			 */
+			ptr = xlate_dev_mem_ptr(p);
+			if (!ptr) {
+				if (written)
+					break;
+				return -EFAULT;
+			}
 
-		copied = copy_from_user(ptr, buf, sz);
-		unxlate_dev_mem_ptr(p, ptr);
-		if (copied) {
-			written += sz - copied;
-			if (written)
-				break;
-			return -EFAULT;
+			copied = copy_from_user(ptr, buf, sz);
+			unxlate_dev_mem_ptr(p, ptr);
+			if (copied) {
+				written += sz - copied;
+				if (written)
+					break;
+				return -EFAULT;
+			}
 		}
 
 		buf += sz;