All of lore.kernel.org
 help / color / mirror / Atom feed
* Patch "metag/uaccess: Fix access_ok()" has been added to the 4.9-stable tree
@ 2017-05-23 14:48 gregkh
  0 siblings, 0 replies; only message in thread
From: gregkh @ 2017-05-23 14:48 UTC (permalink / raw)
  To: james.hogan, gregkh, viro; +Cc: stable, stable-commits


This is a note to let you know that I've just added the patch titled

    metag/uaccess: Fix access_ok()

to the 4.9-stable tree which can be found at:
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
     metag-uaccess-fix-access_ok.patch
and it can be found in the queue-4.9 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@vger.kernel.org> know about it.


>From 8a8b56638bcac4e64cccc88bf95a0f9f4b19a2fb Mon Sep 17 00:00:00 2001
From: James Hogan <james.hogan@imgtec.com>
Date: Fri, 28 Apr 2017 10:50:26 +0100
Subject: metag/uaccess: Fix access_ok()

From: James Hogan <james.hogan@imgtec.com>

commit 8a8b56638bcac4e64cccc88bf95a0f9f4b19a2fb upstream.

The __user_bad() macro used by access_ok() has a few corner cases
noticed by Al Viro where it doesn't behave correctly:

 - The kernel range check has off by 1 errors which permit access to the
   first and last byte of the kernel mapped range.

 - The kernel range check ends at LINCORE_BASE rather than
   META_MEMORY_LIMIT, which is ineffective when the kernel is in global
   space (an extremely uncommon configuration).

There are a couple of other shortcomings here too:

 - Access to the whole of the other address space is permitted (i.e. the
   global half of the address space when the kernel is in local space).
   This isn't ideal as it could theoretically still contain privileged
   mappings set up by the bootloader.

 - The size argument is unused, permitting user copies which start on
   valid pages at the end of the user address range and cross the
   boundary into the kernel address space (e.g. addr = 0x3ffffff0, size
   > 0x10).

It isn't very convenient to add size checks when disallowing certain
regions, and it seems far safer to be sure and explicit about what
userland is able to access, so invert the logic to allow certain regions
instead, and fix the off by 1 errors and missing size checks. This also
allows the get_fs() == KERNEL_DS check to be more easily optimised into
the user address range case.

We now have 3 such allowed regions:

 - The user address range (incorporating the get_fs() == KERNEL_DS
   check).

 - NULL (some kernel code expects this to work, and we'll always catch
   the fault anyway).

 - The core code memory region.

Fixes: 373cd784d0fc ("metag: Memory handling")
Reported-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: linux-metag@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 arch/metag/include/asm/uaccess.h |   40 +++++++++++++++++++++++----------------
 1 file changed, 24 insertions(+), 16 deletions(-)

--- a/arch/metag/include/asm/uaccess.h
+++ b/arch/metag/include/asm/uaccess.h
@@ -28,24 +28,32 @@
 
 #define segment_eq(a, b)	((a).seg == (b).seg)
 
-#define __kernel_ok (segment_eq(get_fs(), KERNEL_DS))
-/*
- * Explicitly allow NULL pointers here. Parts of the kernel such
- * as readv/writev use access_ok to validate pointers, but want
- * to allow NULL pointers for various reasons. NULL pointers are
- * safe to allow through because the first page is not mappable on
- * Meta.
- *
- * We also wish to avoid letting user code access the system area
- * and the kernel half of the address space.
- */
-#define __user_bad(addr, size) (((addr) > 0 && (addr) < META_MEMORY_BASE) || \
-				((addr) > PAGE_OFFSET &&		\
-				 (addr) < LINCORE_BASE))
-
 static inline int __access_ok(unsigned long addr, unsigned long size)
 {
-	return __kernel_ok || !__user_bad(addr, size);
+	/*
+	 * Allow access to the user mapped memory area, but not the system area
+	 * before it. The check extends to the top of the address space when
+	 * kernel access is allowed (there's no real reason to user copy to the
+	 * system area in any case).
+	 */
+	if (likely(addr >= META_MEMORY_BASE && addr < get_fs().seg &&
+		   size <= get_fs().seg - addr))
+		return true;
+	/*
+	 * Explicitly allow NULL pointers here. Parts of the kernel such
+	 * as readv/writev use access_ok to validate pointers, but want
+	 * to allow NULL pointers for various reasons. NULL pointers are
+	 * safe to allow through because the first page is not mappable on
+	 * Meta.
+	 */
+	if (!addr)
+		return true;
+	/* Allow access to core code memory area... */
+	if (addr >= LINCORE_CODE_BASE && addr <= LINCORE_CODE_LIMIT &&
+	    size <= LINCORE_CODE_LIMIT + 1 - addr)
+		return true;
+	/* ... but no other areas. */
+	return false;
 }
 
 #define access_ok(type, addr, size) __access_ok((unsigned long)(addr),	\


Patches currently in stable-queue which might be from james.hogan@imgtec.com are

queue-4.9/metag-uaccess-check-access_ok-in-strncpy_from_user.patch
queue-4.9/metag-uaccess-fix-access_ok.patch

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2017-05-23 14:51 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-23 14:48 Patch "metag/uaccess: Fix access_ok()" has been added to the 4.9-stable tree gregkh

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.