| From: James Hogan <james.hogan@imgtec.com> |
| Date: Fri, 28 Apr 2017 10:50:26 +0100 |
| Subject: metag/uaccess: Fix access_ok() |
| |
| 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: Ben Hutchings <ben@decadent.org.uk> |
| --- |
| 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), \ |