public inbox for linux-arm-kernel@lists.infradead.org 
 help / color / mirror / Atom feed
From: james.morse@arm•com (James Morse)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH v2 0/5] arm64: kernel: Add support for User Access Override
Date: Mon, 07 Mar 2016 16:43:19 +0000	[thread overview]
Message-ID: <56DDAFA7.4090207@arm.com> (raw)
In-Reply-To: <1454684330-892-1-git-send-email-james.morse@arm.com>

Hi Catalin,

I've just spotted UAO causes the test_user_copy module (CONFIG_TEST_USER_COPY)
to fail. Who to blame is up for discussion. The test is passing a user pointer
as the 'to' field of copy_from_user(), which it expects to fail gracefully:

lib/test_user_copy.c:75
>	/* Invalid usage: none of these should succeed. */
[ ... ]
> 	ret |= test(!copy_from_user(bad_usermem, (char __user *)kmem,
>				    PAGE_SIZE),
>		    "illegal reversed copy_from_user passed");
>

access_ok() catches the "(char __user *)kmem", causing copy_from_user() to pass
bad_usermem to memset():

arch/arm64/include/asm/uaccess.h:279
>	if (access_ok(VERIFY_READ, from, n))
>		n = __copy_from_user(to, from, n);
>	else /* security hole - plug it */
>		memset(to, 0, n);

This (correctly) trips UAO's "Accessing user space memory outside uaccess.h
routines" message, which is a little confusing to debug, and stops the rest of
the module's tests from being run.

As far as I can see, this would only affect arm64. I can't find an equivalent
memset() for x86_64.

The below ugly hack [0], handles this more gracefully. I can send this as a fix
sooner/later if you think its the right thing to do.



Thanks,

James

[0]
-----------------%<-----------------
diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
index 0685d74572af..049a82e8dd9e 100644
--- a/arch/arm64/include/asm/uaccess.h
+++ b/arch/arm64/include/asm/uaccess.h
@@ -278,8 +278,8 @@ static inline unsigned long __must_check copy_from_user(void
*to, const void __u
 {
        if (access_ok(VERIFY_READ, from, n))
                n = __copy_from_user(to, from, n);
-       else /* security hole - plug it */
-               memset(to, 0, n);
+       else if ((unsigned long)to > USER_DS) /* swapped from/to args? */
+               memset(to, 0, n); /* security hole - plug it */
        return n;
 }

-----------------%<-----------------

  parent reply	other threads:[~2016-03-07 16:43 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-05 14:58 [PATCH v2 0/5] arm64: kernel: Add support for User Access Override James Morse
2016-02-05 14:58 ` [PATCH v2 1/5] arm64: cpufeature: Change read_cpuid() to use sysreg's mrs_s macro James Morse
2016-02-05 14:58 ` [PATCH v2 2/5] arm64: add ARMv8.2 id_aa64mmfr2 boiler plate James Morse
2016-03-03 17:59   ` Christopher Covington
2016-03-03 18:27     ` Robin Murphy
2016-03-03 19:03       ` Christopher Covington
2016-03-03 19:19         ` Will Deacon
2016-03-04 10:20           ` Suzuki K. Poulose
2016-03-04 13:37             ` James Morse
2016-03-04 13:54               ` Robin Murphy
2016-03-04 14:59         ` Mark Rutland
2016-03-04 18:15           ` Christopher Covington
2016-02-05 14:58 ` [PATCH v2 3/5] arm64: kernel: Add support for User Access Override James Morse
2016-02-18 12:26   ` Catalin Marinas
2016-02-05 14:58 ` [PATCH v2 4/5] arm64: cpufeature: Test 'matches' pointer to find the end of the list James Morse
2016-02-05 14:58 ` [PATCH v2 5/5] arm64: kernel: Don't toggle PAN on systems with UAO James Morse
2016-02-18 14:36   ` Catalin Marinas
2016-02-18 14:43     ` James Morse
2016-02-05 15:40 ` [PATCH v2 0/5] arm64: kernel: Add support for User Access Override Arnd Bergmann
2016-02-09  9:47   ` Will Deacon
2016-02-18 18:03 ` Catalin Marinas
2016-02-19 15:38   ` Peter Maydell
2016-02-19 16:46     ` Catalin Marinas
2016-02-19 16:54       ` Peter Maydell
2016-02-19 16:57         ` Ard Biesheuvel
2016-02-19 17:03           ` Peter Maydell
2016-03-07 16:43 ` James Morse [this message]
2016-03-07 17:23   ` Russell King - ARM Linux
2016-03-07 17:40     ` James Morse
2016-03-07 17:51       ` Russell King - ARM Linux
2016-03-07 17:38   ` Catalin Marinas
2016-03-07 20:54     ` Kees Cook
2016-03-08 17:19       ` Catalin Marinas
2016-03-08 17:39         ` Kees Cook
2016-03-08 18:22           ` Catalin Marinas
2016-03-08 18:27             ` Kees Cook

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=56DDAFA7.4090207@arm.com \
    --to=james.morse@arm$(echo .)com \
    --cc=linux-arm-kernel@lists$(echo .)infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox