LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v2 08/38] cxlflash: Read host AFU configuration
From: Andrew Donnellan @ 2018-03-09  4:10 UTC (permalink / raw)
  To: Uma Krishnan, linux-scsi, James Bottomley, Martin K. Petersen,
	Matthew R. Ochs, Manoj N. Kumar
  Cc: linuxppc-dev, Frederic Barrat, Christophe Lombard
In-Reply-To: <1519683660-17039-1-git-send-email-ukrishn@linux.vnet.ibm.com>

On 27/02/18 09:21, Uma Krishnan wrote:
> The host AFU configuration is read on the initialization path to identify
> the features and configuration of the AFU. This data is cached for use in
> later configuration steps.
> 
> Signed-off-by: Uma Krishnan <ukrishn@linux.vnet.ibm.com>
> Acked-by: Matthew R. Ochs <mrochs@linux.vnet.ibm.com>

Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>

-- 
Andrew Donnellan              OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com  IBM Australia Limited

^ permalink raw reply

* Re: [PATCH v2 09/38] cxlflash: Setup AFU acTag range
From: Andrew Donnellan @ 2018-03-09  4:19 UTC (permalink / raw)
  To: Uma Krishnan, linux-scsi, James Bottomley, Martin K. Petersen,
	Matthew R. Ochs, Manoj N. Kumar
  Cc: linuxppc-dev, Frederic Barrat, Christophe Lombard
In-Reply-To: <1519683669-17075-1-git-send-email-ukrishn@linux.vnet.ibm.com>

On 27/02/18 09:21, Uma Krishnan wrote:
> The OCXL specification supports distributing acTags amongst different
> AFUs and functions on the link. As cxlflash devices are expected to only
> support a single AFU and function, the entire range that was assigned to
> the function is also assigned to the AFU.
> 
> Signed-off-by: Uma Krishnan <ukrishn@linux.vnet.ibm.com>
> Acked-by: Matthew R. Ochs <mrochs@linux.vnet.ibm.com>

Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>

> diff --git a/drivers/scsi/cxlflash/ocxl_hw.h b/drivers/scsi/cxlflash/ocxl_hw.h
> index f6af247..9c675fa 100644
> --- a/drivers/scsi/cxlflash/ocxl_hw.h
> +++ b/drivers/scsi/cxlflash/ocxl_hw.h
> @@ -22,4 +22,6 @@ struct ocxl_hw_afu {
> 
>   	int fn_actag_base;		/* Function acTag base */
>   	int fn_actag_enabled;		/* Function acTag number enabled */
> +	int afu_actag_base;		/* AFU acTag base */
> +	int afu_actag_enabled;		/* AFU acTag number enabled */
>   };
> 

Is it ever foreseen that you will need to support more than one AFU on a 
function? I guess it's good to keep the fn and afu values separate 
anyway just in case...

-- 
Andrew Donnellan              OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com  IBM Australia Limited

^ permalink raw reply

* Re: [PATCH v2 10/38] cxlflash: Setup AFU PASID
From: Andrew Donnellan @ 2018-03-09  4:29 UTC (permalink / raw)
  To: Uma Krishnan, linux-scsi, James Bottomley, Martin K. Petersen,
	Matthew R. Ochs, Manoj N. Kumar
  Cc: linuxppc-dev, Frederic Barrat, Christophe Lombard
In-Reply-To: <1519683693-17113-1-git-send-email-ukrishn@linux.vnet.ibm.com>

On 27/02/18 09:21, Uma Krishnan wrote:
> Per the OCXL specification, the maximum PASID supported by the AFU is
> indicated by a field within the configuration space. Similar to acTags,
> implementations can choose to use any sub-range of PASID within their
> assigned range. For cxlflash, the entire range is used.
> 
> Signed-off-by: Uma Krishnan <ukrishn@linux.vnet.ibm.com>
> Acked-by: Matthew R. Ochs <mrochs@linux.vnet.ibm.com>

Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>

-- 
Andrew Donnellan              OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com  IBM Australia Limited

^ permalink raw reply

* Re: [PATCH v2 00/38] cxlflash: OCXL transport support
From: Andrew Donnellan @ 2018-03-09  5:08 UTC (permalink / raw)
  To: Martin K. Petersen, Uma Krishnan
  Cc: linux-scsi, James Bottomley, Matthew R. Ochs, Manoj N. Kumar,
	linuxppc-dev, Frederic Barrat, Christophe Lombard
In-Reply-To: <yq18tb5rskt.fsf@oracle.com>

On 07/03/18 05:01, Martin K. Petersen wrote:
> 
>> This patch series adds OCXL support to the cxlflash driver. With this
>> support, new devices using the OCXL transport will be supported by the
>> cxlflash driver along with the existing CXL devices. An effort is made
>> to keep this transport specific function independent of the existing
>> core driver that communicates with the AFU.
> 
> This one has been sitting for a week without any reviews. Andrew, when
> will you have time to take a look? >
> Being a pretty big chunk of code, I would like to merge it sooner rather
> than later. Or we can postpone until 4.18, of course.
> 

Slowly getting through it, though as of next Friday I'll be disappearing 
from the face of the earth for 3 weeks and I'm not sure I'll be finished 
by then.

Fred: could you take a look at this series if you have spare cycles?

-- 
Andrew Donnellan              OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com  IBM Australia Limited

^ permalink raw reply

* Re: [bug?] Access was denied by memory protection keys in execute-only address
From: Ram Pai @ 2018-03-09  5:50 UTC (permalink / raw)
  To: Li Wang
  Cc: Michael Ellerman, Jan Stancek, ltp, linux-mm,
	linuxppc-dev@lists.ozlabs.org
In-Reply-To: <CAEemH2czWDjvJLpL6ynV1+VxCFh_-A-d72tJhA5zwgrAES2nWA@mail.gmail.com>

On Fri, Mar 09, 2018 at 11:43:00AM +0800, Li Wang wrote:
>    On Fri, Mar 9, 2018 at 12:45 AM, Ram Pai <[1]linuxram@us.ibm.com> wrote:
> 
>      On Thu, Mar 08, 2018 at 11:19:12PM +1100, Michael Ellerman wrote:
>      > Li Wang <[2]liwang@redhat.com> writes:
>      > > Hi,
>      > >
>      > > ltp/mprotect04[1] crashed by SEGV_PKUERR on ppc64(LPAR on P730,
>      Power 8
>      > > 8247-22L) with kernel-v4.16.0-rc4.
>      > >
>      > > 10000000-10020000 r-xp 00000000 fd:00 167223           mprotect04
>      > > 10020000-10030000 r--p 00010000 fd:00 167223           mprotect04
>      > > 10030000-10040000 rw-p 00020000 fd:00 167223           mprotect04
>      > > 1001a380000-1001a3b0000 rw-p 00000000 00:00 0          [heap]
>      > > 7fffa6c60000-7fffa6c80000 --xp 00000000 00:00 0 ​
>      > >
>      > > ​&exec_func = 0x10030170​
>      > >
>      > > ​&func = 0x7fffa6c60170​
>      > >
>      > > ​While perform ​
>      > > "(*func)();" we get the
>      > > ​segmentation fault.
>      > > ​
>      > >
>      > > ​strace log:​
>      > >
>      > > -------------------
>      > > ​mprotect(0x7fffaed00000, 131072, PROT_EXEC) = 0
>      > > rt_sigprocmask(SIG_BLOCK, NULL, [], 8)  = 0
>      > > --- SIGSEGV {si_signo=SIGSEGV, si_code=SEGV_PKUERR,
>      si_addr=0x7fffaed00170}
>      > > ---​
>      >
>      > Looks like a bug to me.
>      >
>      > Please Cc linuxppc-dev on powerpc bugs.
>      >
>      > I also can't reproduce this failure on my machine.
>      > Not sure what's going on?
> 
>      I could reproduce it on a power7 lpar.  But not on a power8 lpar.
> 
>>      The problem seems to be that the cpu generates a key exception if
>>      the page with Read/Write-disable-but-execute-enable key is executed
>>      on power7. If I enable read on that key, the exception disappears.
>> 
>    After adding read permission on that key, reproducer get PASS on my power8
>    machine too.​
>    ​(​mprotect(..,PROT_READ | PROT_EXEC))​

I enabled READ permission on the key by resetting the bit in
the AMR register.  And that healed the problem. So the point is
something is erroneously triggering the MMU to generate a key-exception
if the READ permission on the key is disabled.

>     
> 
>      BTW: the testcase executes
>      ​​mprotect(..,PROT_EXEC).
>      The mprotect(, PROT_EXEC) system call internally generates a
>      execute-only key and associates it with the pages in the address-range. 
> 
>      Now since Li Wang claims that he can reproduce it on power8 as well, i
>      am wondering if the slightly different cpu behavior is dependent on the
>      version of the firmware/microcode?
> 
>    ​I also run this reproducer on series ppc kvm machines, but none of them
>    get the FAIL.

on ppc kvm virtual-machines the pkey subsystem is not entirely enabled
yet. Though the kernel code exists it does not get enabled, since the
feature is not yet exported in the device-tree by qemu. 

>    If you need some more HW info, pls let me know.​

Will do thanks.
RP

^ permalink raw reply

* Re: [PATCH] cxl: Perform NULL check for 'cxl_afu *' at various places in cxl
From: Andrew Donnellan @ 2018-03-09  6:02 UTC (permalink / raw)
  To: Vaibhav Jain, linuxppc-dev, Frederic Barrat
  Cc: Philippe Bergheaud, Alastair D'Silva, Christophe Lombard
In-Reply-To: <874llqosxq.fsf@vajain21.in.ibm.com>

On 09/03/18 13:59, Vaibhav Jain wrote:
> Thanks for looking into this patch Andrew,
> 
> Andrew Donnellan <andrew.donnellan@au1.ibm.com> writes:
> 
>> On 08/03/18 21:05, Vaibhav Jain wrote:
>>> It is possible for a CXL card to have a valid PSL but no valid
>>> AFUs. When this happens we have a valid instance of 'struct cxl'
>>> representing the adapter but with its member 'struct cxl_afu *cxl[]'
>>> as empty. Unfortunately at many placed within cxl code (especially
>>> during an EEH) the elements of this array are passed on to various
>>> other cxl functions. Which may result in kernel oops/panic when this
>>> 'struct cxl_afu *' is dereferenced.
>>>
>>> So this patch puts a NULL check at the beginning of various cxl
>>> functions that accept 'struct cxl_afu *' as a formal argument and are
>>> called from with a loop of the form:
>>>
>>>          for (i = 0; i < adapter->slices; i++) {
>>>          	   	afu = adapter->afu[i];
>>> 		/* call some function with 'afu' */
>>> 	}
>>
>> Surely in this case adapter->slices should be 0?
> Not necessarily, as adapter->slice doesnt take into account AFUs that
> fail to init. I saw this issue in one specific case were the only slice
> on the card had issued with the AFU descriptor caused CXL init of that
> AFU to fail.

Ah OK, makes sense.


-- 
Andrew Donnellan              OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com  IBM Australia Limited

^ permalink raw reply

* [PATCH] x86, powerpc : pkey-mprotect must allow pkey-0
From: Ram Pai @ 2018-03-09  8:12 UTC (permalink / raw)
  To: mpe, mingo, akpm
  Cc: linuxppc-dev, linux-mm, x86, linux-arch, linux-kernel,
	dave.hansen, benh, paulus, khandual, aneesh.kumar, bsingharora,
	hbabu, mhocko, bauerman, ebiederm, linuxram, corbet, arnd,
	fweimer, msuchanek, Ulrich.Weigand

Once an address range is associated with an allocated pkey, it cannot be
reverted back to key-0. There is no valid reason for the above behavior.  On
the contrary applications need the ability to do so.

The patch relaxes the restriction.

Tested on powerpc and x86_64.

cc: Dave Hansen <dave.hansen@intel.com>
cc: Michael Ellermen <mpe@ellerman.id.au>
cc: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
 arch/powerpc/include/asm/pkeys.h | 19 ++++++++++++++-----
 arch/x86/include/asm/pkeys.h     |  5 +++--
 2 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
index 0409c80..3e8abe4 100644
--- a/arch/powerpc/include/asm/pkeys.h
+++ b/arch/powerpc/include/asm/pkeys.h
@@ -101,10 +101,18 @@ static inline u16 pte_to_pkey_bits(u64 pteflags)
 
 static inline bool mm_pkey_is_allocated(struct mm_struct *mm, int pkey)
 {
-	/* A reserved key is never considered as 'explicitly allocated' */
-	return ((pkey < arch_max_pkey()) &&
-		!__mm_pkey_is_reserved(pkey) &&
-		__mm_pkey_is_allocated(mm, pkey));
+	/* pkey 0 is allocated by default. */
+	if (!pkey)
+	       return true;
+
+	if (pkey < 0 || pkey >= arch_max_pkey())
+	       return false;
+
+	/* reserved keys are never allocated. */
+	if (__mm_pkey_is_reserved(pkey))
+	       return false;
+
+	return(__mm_pkey_is_allocated(mm, pkey));
 }
 
 extern void __arch_activate_pkey(int pkey);
@@ -150,7 +158,8 @@ static inline int mm_pkey_free(struct mm_struct *mm, int pkey)
 	if (static_branch_likely(&pkey_disabled))
 		return -1;
 
-	if (!mm_pkey_is_allocated(mm, pkey))
+	/* pkey 0 cannot be freed */
+	if (!pkey || !mm_pkey_is_allocated(mm, pkey))
 		return -EINVAL;
 
 	/*
diff --git a/arch/x86/include/asm/pkeys.h b/arch/x86/include/asm/pkeys.h
index a0ba1ff..6ea7486 100644
--- a/arch/x86/include/asm/pkeys.h
+++ b/arch/x86/include/asm/pkeys.h
@@ -52,7 +52,7 @@ bool mm_pkey_is_allocated(struct mm_struct *mm, int pkey)
 	 * from pkey_alloc().  pkey 0 is special, and never
 	 * returned from pkey_alloc().
 	 */
-	if (pkey <= 0)
+	if (pkey < 0)
 		return false;
 	if (pkey >= arch_max_pkey())
 		return false;
@@ -92,7 +92,8 @@ int mm_pkey_alloc(struct mm_struct *mm)
 static inline
 int mm_pkey_free(struct mm_struct *mm, int pkey)
 {
-	if (!mm_pkey_is_allocated(mm, pkey))
+	/* pkey 0 is special and can never be freed */
+	if (!pkey || !mm_pkey_is_allocated(mm, pkey))
 		return -EINVAL;
 
 	mm_set_pkey_free(mm, pkey);
-- 
1.8.3.1

^ permalink raw reply related

* Re: [RFC PATCH 1/1] powerpc/ftrace: Exclude real mode code from
From: Naveen N. Rao @ 2018-03-09  8:15 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Michael Ellerman, Paul Mackerras,
	Steven Rostedt
  Cc: Anton Blanchard, linuxppc-dev, Nicholas Piggin
In-Reply-To: <87efkve0a2.fsf@concordia.ellerman.id.au>

Michael Ellerman wrote:
> "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> writes:
>=20
>> We can't take a trap in most parts of real mode code. Instead of adding
>> the 'notrace' annotation to all C functions that can be invoked from
>> real mode, detect that we are in real mode on ftrace entry and return
>> back.
>>
>> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
>> ---
>> This RFC only handles -mprofile-kernel to demonstrate the approach being=
=20
>> considered. We will need to handle other ftrace entry if we decide to=20
>> continue down this path.
>=20
> Paul and I were talking about having a paca flag for this, ie.
> paca->safe_to_ftrace (or whatever). I'm not sure if you've talked to
> him and decided this is a better approach.
>=20
> I guess I'm 50/50 on which is better, they both have pluses and minuses.

Thanks, I hadn't spoken to Paul, but I now think that this is probably=20
the better approach to take.

My earlier assumption was that we have other scenarios when we are in=20
realmode (specifically with MSR_RI unset) where we won't be able to=20
recover from a trap, during function tracing (*). I did a set of=20
experiments yesterday to verify that, but I was not able to uncover any=20
such scenarios with my brief testing. So, we seem to be functioning just=20
fine while tracing realmode C code, except for KVM.

As such, rather than blacklisting all realmode code, I think it is=20
better to be selective and just disable the tracer for KVM since we know=20
we can't take a trap there. We will be able to use the same approach if=20
we uncover additional scenarios where we can't use function tracing. I=20
will look at implementing a paca field for this purpose.

I also noticed that even with an unexpected timebase, we still seem to=20
recover just fine with a simple change:

--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -2629,8 +2629,8 @@ static noinline void
 rb_handle_timestamp(struct ring_buffer_per_cpu *cpu_buffer,
                    struct rb_event_info *info)
 {
-       WARN_ONCE(info->delta > (1ULL << 59),
-                 KERN_WARNING "Delta way too big! %llu ts=3D%llu write sta=
mp =3D %llu\n%s",
+       if (info->delta > (1ULL << 59))
+               pr_warn_once("Delta way too big! %llu ts=3D%llu write stamp=
 =3D %llu\n%s",
                  (unsigned long long)info->delta,
                  (unsigned long long)info->ts,
                  (unsigned long long)cpu_buffer->write_stamp,


This allowed the virtual machine to boot and we were able to trace the=20
rest of KVM C code. I only just did a boot test, so I'm not sure if=20
there are other scenarios where things can go wrong.

Steve,
Would you be willing to accept a patch like the above? Since we seem to=20
handle the larger delta just fine, I think the above change should be=20
fine?

I will still work on excluding KVM C code from being traced, but the=20
advantage with the above patch is that we will be able to trace KVM C=20
code with a small change if necessary.


- Naveen

---
(*) putting on my kprobe hat

=

^ permalink raw reply

* Re: [RFC PATCH 1/1] powerpc/ftrace: Exclude real mode code from
From: Naveen N. Rao @ 2018-03-09  8:17 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Anton Blanchard, Benjamin Herrenschmidt, linuxppc-dev,
	Michael Ellerman, Nicholas Piggin, Paul Mackerras
In-Reply-To: <20180307140232.0023b8cc@vmware.local.home>

Steven Rostedt wrote:
> On Thu, 08 Mar 2018 00:07:07 +0530
> "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:
>=20
>> Yes, that's negligible.
>> Though, to be honest, I will have to introduce a 'mfmsr' for the older=20
>> -pg variant. I still think that the improved reliability far outweighs=20
>> the minor slowdown there.
>=20
> In that case, can you introduce a read_mostly variable that can be
> tested before calling the mfmsr. Why punish normal ftrace tracing if
> kvm is not enabled or running?
>=20
> Both should probably have an #ifdef CONFIG_KVM encapsulating the code.

Agreed. My previous intent was to exclude all realmode code from ftrace,=20
but I now think it is better to only exclude the KVM path. So, I will=20
make it specific to KVM.

- Naveen

=

^ permalink raw reply

* Re: [PATCH] x86, powerpc : pkey-mprotect must allow pkey-0
From: Balbir Singh @ 2018-03-09  8:37 UTC (permalink / raw)
  To: Ram Pai
  Cc: Michael Ellerman, Ingo Molnar, akpm@linux-foundation.org,
	open list:LINUX FOR POWERPC (32-BIT AND 64-BIT), linux-mm,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), linux-arch,
	linux-kernel@vger.kernel.org, Dave Hansen, Benjamin Herrenschmidt,
	Paul Mackerras, Anshuman Khandual, Aneesh Kumar KV,
	Haren Myneni/Beaverton/IBM, Michal Hocko, Thiago Jung Bauermann,
	Eric W. Biederman, Jonathan Corbet, Arnd Bergmann, fweimer,
	msuchanek, Ulrich.Weigand
In-Reply-To: <1520583161-11741-1-git-send-email-linuxram@us.ibm.com>

On Fri, Mar 9, 2018 at 7:12 PM, Ram Pai <linuxram@us.ibm.com> wrote:
> Once an address range is associated with an allocated pkey, it cannot be
> reverted back to key-0. There is no valid reason for the above behavior.  On
> the contrary applications need the ability to do so.
>
> The patch relaxes the restriction.

I looked at the code and my observation was going to be that we need
to change mm_pkey_is_allocated. I still fail to understand what
happens if pkey 0 is reserved? What is the default key is it the first
available key? Assuming 0 is the default key may work and seems to
work, but I am sure its mostly by accident. It would be nice, if we
could have  a notion of the default key. I don't like the special
meaning given to key 0 here. Remember on powerpc if 0 is reserved and
UAMOR/AMOR does not allow modification because it's reserved, setting
0 will still fail

Balbir

^ permalink raw reply

* Re: [PATCH] x86, powerpc : pkey-mprotect must allow pkey-0
From: Ingo Molnar @ 2018-03-09  8:43 UTC (permalink / raw)
  To: Ram Pai
  Cc: mpe, mingo, akpm, linuxppc-dev, linux-mm, x86, linux-arch,
	linux-kernel, dave.hansen, benh, paulus, khandual, aneesh.kumar,
	bsingharora, hbabu, mhocko, bauerman, ebiederm, corbet, arnd,
	fweimer, msuchanek, Ulrich.Weigand
In-Reply-To: <1520583161-11741-1-git-send-email-linuxram@us.ibm.com>


* Ram Pai <linuxram@us.ibm.com> wrote:

> Once an address range is associated with an allocated pkey, it cannot be
> reverted back to key-0. There is no valid reason for the above behavior.  On
> the contrary applications need the ability to do so.
> 
> The patch relaxes the restriction.
> 
> Tested on powerpc and x86_64.
> 
> cc: Dave Hansen <dave.hansen@intel.com>
> cc: Michael Ellermen <mpe@ellerman.id.au>
> cc: Ingo Molnar <mingo@kernel.org>
> Signed-off-by: Ram Pai <linuxram@us.ibm.com>
> ---
>  arch/powerpc/include/asm/pkeys.h | 19 ++++++++++++++-----
>  arch/x86/include/asm/pkeys.h     |  5 +++--
>  2 files changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
> index 0409c80..3e8abe4 100644
> --- a/arch/powerpc/include/asm/pkeys.h
> +++ b/arch/powerpc/include/asm/pkeys.h
> @@ -101,10 +101,18 @@ static inline u16 pte_to_pkey_bits(u64 pteflags)
>  
>  static inline bool mm_pkey_is_allocated(struct mm_struct *mm, int pkey)
>  {
> -	/* A reserved key is never considered as 'explicitly allocated' */
> -	return ((pkey < arch_max_pkey()) &&
> -		!__mm_pkey_is_reserved(pkey) &&
> -		__mm_pkey_is_allocated(mm, pkey));
> +	/* pkey 0 is allocated by default. */
> +	if (!pkey)
> +	       return true;
> +
> +	if (pkey < 0 || pkey >= arch_max_pkey())
> +	       return false;
> +
> +	/* reserved keys are never allocated. */
> +	if (__mm_pkey_is_reserved(pkey))
> +	       return false;

Please capitalize in comments consistently, i.e.:

	/* Reserved keys are never allocated: */

> +
> +	return(__mm_pkey_is_allocated(mm, pkey));

'return' is not a function.

Thanks,

	Ingo

^ permalink raw reply

* Re: [PATCH 00/27] cpufreq: Stop validating cpufreq table in drivers
From: Viresh Kumar @ 2018-03-09 10:00 UTC (permalink / raw)
  To: Rafael Wysocki, bcm-kernel-feedback-list, Benjamin Herrenschmidt,
	Brian Norris, Florian Fainelli, Gregory Fong, Jonathan Hunter,
	Krzysztof Kozlowski, Kukjin Kim, Markus Mayer, Matthias Brugger,
	Michael Ellerman, Paul Mackerras, Sudeep Holla, Thierry Reding
  Cc: linux-pm, Vincent Guittot, linux-arm-kernel, linux-mediatek,
	linuxppc-dev, linux-samsung-soc, linux-tegra
In-Reply-To: <cover.1519620578.git.viresh.kumar@linaro.org>

On 26-02-18, 10:38, Viresh Kumar wrote:
> Hi,
> 
> A patchset [1] sent last week already updated the cpufreq core to start
> validating cpufreq table if the policy contains a valid
> "policy->freq_table" pointer.
> 
> This series updates all such drivers to stop validating the cpufreq
> table directly and let only the core handle it.
> 
> It depends on the previous series [1] and two cleanup patches [2][3] and
> is rebased over 4.16-rc3.
> 
> It is already tested by the 0-day testing infrastructure and no issues
> were reported.

We will assume no objections or concerns if you (cc'd here and on the individual
commits) don't respond and the series will then get merged into linux-next next
week.

Thanks.

--
viresh

^ permalink raw reply

* Re: [PATCH] x86, powerpc : pkey-mprotect must allow pkey-0
From: Michael Ellerman @ 2018-03-09 10:19 UTC (permalink / raw)
  To: Ram Pai, mingo, akpm
  Cc: linuxppc-dev, linux-mm, x86, linux-arch, linux-kernel,
	dave.hansen, benh, paulus, khandual, aneesh.kumar, bsingharora,
	hbabu, mhocko, bauerman, ebiederm, linuxram, corbet, arnd,
	fweimer, msuchanek, Ulrich.Weigand
In-Reply-To: <1520583161-11741-1-git-send-email-linuxram@us.ibm.com>

Ram Pai <linuxram@us.ibm.com> writes:

> Once an address range is associated with an allocated pkey, it cannot be
> reverted back to key-0. There is no valid reason for the above behavior.  On
> the contrary applications need the ability to do so.

Please explain this in much more detail. Is it an ABI change?

And why did we just notice this?

> The patch relaxes the restriction.
>
> Tested on powerpc and x86_64.

Thanks, but please split the patch, one for each arch.

cheers

> diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
> index 0409c80..3e8abe4 100644
> --- a/arch/powerpc/include/asm/pkeys.h
> +++ b/arch/powerpc/include/asm/pkeys.h
> @@ -101,10 +101,18 @@ static inline u16 pte_to_pkey_bits(u64 pteflags)
>  
>  static inline bool mm_pkey_is_allocated(struct mm_struct *mm, int pkey)
>  {
> -	/* A reserved key is never considered as 'explicitly allocated' */
> -	return ((pkey < arch_max_pkey()) &&
> -		!__mm_pkey_is_reserved(pkey) &&
> -		__mm_pkey_is_allocated(mm, pkey));
> +	/* pkey 0 is allocated by default. */
> +	if (!pkey)
> +	       return true;
> +
> +	if (pkey < 0 || pkey >= arch_max_pkey())
> +	       return false;
> +
> +	/* reserved keys are never allocated. */
> +	if (__mm_pkey_is_reserved(pkey))
> +	       return false;
> +
> +	return(__mm_pkey_is_allocated(mm, pkey));
>  }
>  
>  extern void __arch_activate_pkey(int pkey);
> @@ -150,7 +158,8 @@ static inline int mm_pkey_free(struct mm_struct *mm, int pkey)
>  	if (static_branch_likely(&pkey_disabled))
>  		return -1;
>  
> -	if (!mm_pkey_is_allocated(mm, pkey))
> +	/* pkey 0 cannot be freed */
> +	if (!pkey || !mm_pkey_is_allocated(mm, pkey))
>  		return -EINVAL;
>  
>  	/*
> diff --git a/arch/x86/include/asm/pkeys.h b/arch/x86/include/asm/pkeys.h
> index a0ba1ff..6ea7486 100644
> --- a/arch/x86/include/asm/pkeys.h
> +++ b/arch/x86/include/asm/pkeys.h
> @@ -52,7 +52,7 @@ bool mm_pkey_is_allocated(struct mm_struct *mm, int pkey)
>  	 * from pkey_alloc().  pkey 0 is special, and never
>  	 * returned from pkey_alloc().
>  	 */
> -	if (pkey <= 0)
> +	if (pkey < 0)
>  		return false;
>  	if (pkey >= arch_max_pkey())
>  		return false;
> @@ -92,7 +92,8 @@ int mm_pkey_alloc(struct mm_struct *mm)
>  static inline
>  int mm_pkey_free(struct mm_struct *mm, int pkey)
>  {
> -	if (!mm_pkey_is_allocated(mm, pkey))
> +	/* pkey 0 is special and can never be freed */
> +	if (!pkey || !mm_pkey_is_allocated(mm, pkey))
>  		return -EINVAL;
>  
>  	mm_set_pkey_free(mm, pkey);
> -- 
> 1.8.3.1

^ permalink raw reply

* Re: [RFC PATCH 1/1] powerpc/ftrace: Exclude real mode code from
From: Michael Ellerman @ 2018-03-09 10:27 UTC (permalink / raw)
  To: Naveen N. Rao, Benjamin Herrenschmidt, Paul Mackerras,
	Steven Rostedt
  Cc: Anton Blanchard, linuxppc-dev, Nicholas Piggin
In-Reply-To: <1520581934.wrzeu3w6z6.naveen@linux.ibm.com>

"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> writes:

> Michael Ellerman wrote:
>> "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> writes:
>> 
>>> We can't take a trap in most parts of real mode code. Instead of adding
>>> the 'notrace' annotation to all C functions that can be invoked from
>>> real mode, detect that we are in real mode on ftrace entry and return
>>> back.
>>>
>>> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
>>> ---
>>> This RFC only handles -mprofile-kernel to demonstrate the approach being 
>>> considered. We will need to handle other ftrace entry if we decide to 
>>> continue down this path.
>> 
>> Paul and I were talking about having a paca flag for this, ie.
>> paca->safe_to_ftrace (or whatever). I'm not sure if you've talked to
>> him and decided this is a better approach.
>> 
>> I guess I'm 50/50 on which is better, they both have pluses and minuses.
>
> Thanks, I hadn't spoken to Paul, but I now think that this is probably 
> the better approach to take.

OK.

> My earlier assumption was that we have other scenarios when we are in 
> realmode (specifically with MSR_RI unset) where we won't be able to 
> recover from a trap, during function tracing (*). I did a set of 
> experiments yesterday to verify that, but I was not able to uncover any 
> such scenarios with my brief testing. So, we seem to be functioning just 
> fine while tracing realmode C code, except for KVM.

Hmm. If MSR_RI is clear then that should indicate that you can't recover
from an interrupt, typically because you'd lose state in SRR0/1. So I
would expect things to go badly in that case.

But that's sort of orthogonal to real mode. Real mode is different and
we do need to be careful, but a blanket ban on tracing in real mode
might be too broad.

The problem with KVM is that you're running in real mode (MMU off), in
the host kernel, but with the MMU context of the guest loaded. So if you
do anything that turns the MMU on, like a WARN_ON(), then all of a
sudden you're running guest kernel code.

> As such, rather than blacklisting all realmode code, I think it is 
> better to be selective and just disable the tracer for KVM since we know 
> we can't take a trap there. We will be able to use the same approach if 
> we uncover additional scenarios where we can't use function tracing. I 
> will look at implementing a paca field for this purpose.

Thanks. I think it could work well.

We could also use it during early boot, ie. the flag starts out false,
and in the kexec/kdump path as well.

cheers

^ permalink raw reply

* [GIT PULL] Please pull powerpc/linux.git powerpc-4.16-5 tag
From: Michael Ellerman @ 2018-03-09 10:54 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: alastair, andrew.donnellan, bharata, fbarrat, linux-kernel, linux,
	linuxppc-dev

[-- Attachment #1: Type: text/plain, Size: 2320 bytes --]

Hi Linus,

Please pull some more powerpc fixes for 4.16:

The following changes since commit eb0a2d2620ae431c543963c8c7f08f597366fc60:

  powerpc/powernv: Support firmware disable of RFI flush (2018-02-23 16:55:42 +1100)

are available in the git repository at:

  https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git tags/powerpc-4.16-5

for you to fetch changes up to b0c41b8b6e43120d7c35e4709508a3d90a09646e:

  powerpc/pseries: Fix vector5 in ibm architecture vector table (2018-03-06 23:05:38 +1100)

----------------------------------------------------------------
powerpc fixes for 4.16 #5

One notable fix to properly advertise our support for a new firmware feature,
caused by two series conflicting semantically but not textually.

There's a new ioctl for the new ocxl driver, which is not a fix, but needed to
complete the userspace API and good to have before the driver is in a released
kernel.

Finally three minor selftest fixes, and a fix for intermittent build failures
for some obscure platforms, caused by a missing make dependency.

Thanks to:
  Alastair D'Silva, Bharata B Rao, Guenter Roeck.

----------------------------------------------------------------
Alastair D'Silva (2):
      ocxl: Add get_metadata IOCTL to share OCXL information to userspace
      ocxl: Document the OCXL_IOCTL_GET_METADATA IOCTL

Bharata B Rao (1):
      powerpc/pseries: Fix vector5 in ibm architecture vector table

Guenter Roeck (1):
      powerpc/boot: Fix random libfdt related build errors

Michael Ellerman (3):
      selftests/powerpc: Skip tm-trap if transactional memory is not enabled
      selftests/powerpc: Fix missing clean of pmu/lib.o
      selftests/powerpc: Skip the subpage_prot tests if the syscall is unavailable

 Documentation/accelerators/ocxl.rst               |  5 +++++
 arch/powerpc/boot/Makefile                        |  3 ++-
 arch/powerpc/kernel/prom_init.c                   |  1 -
 drivers/misc/ocxl/file.c                          | 27 +++++++++++++++++++++++
 include/uapi/misc/ocxl.h                          | 17 ++++++++++++++
 tools/testing/selftests/powerpc/mm/subpage_prot.c | 14 ++++++++++++
 tools/testing/selftests/powerpc/tm/Makefile       |  2 +-
 tools/testing/selftests/powerpc/tm/tm-trap.c      |  2 ++
 8 files changed, 68 insertions(+), 3 deletions(-)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 800 bytes --]

^ permalink raw reply

* Re: [PATCH] x86, powerpc : pkey-mprotect must allow pkey-0
From: Florian Weimer @ 2018-03-09 11:04 UTC (permalink / raw)
  To: Ram Pai, mpe, mingo, akpm
  Cc: linuxppc-dev, linux-mm, x86, linux-arch, linux-kernel,
	dave.hansen, benh, paulus, khandual, aneesh.kumar, bsingharora,
	hbabu, mhocko, bauerman, ebiederm, corbet, arnd, msuchanek,
	Ulrich.Weigand
In-Reply-To: <1520583161-11741-1-git-send-email-linuxram@us.ibm.com>

On 03/09/2018 09:12 AM, Ram Pai wrote:
> Once an address range is associated with an allocated pkey, it cannot be
> reverted back to key-0. There is no valid reason for the above behavior.

mprotect without a key does not necessarily use key 0, e.g. if 
protection keys are used to emulate page protection flag combination 
which is not directly supported by the hardware.

Therefore, it seems to me that filtering out non-allocated keys is the 
right thing to do.

Thanks,
Florian

^ permalink raw reply

* Re: [RFC PATCH 1/1] powerpc/ftrace: Exclude real mode code from
From: Naveen N. Rao @ 2018-03-09 12:05 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Michael Ellerman, Paul Mackerras,
	Steven Rostedt
  Cc: Anton Blanchard, linuxppc-dev, Nicholas Piggin
In-Reply-To: <87ina5v91g.fsf@concordia.ellerman.id.au>

Michael Ellerman wrote:
> "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> writes:
>> My earlier assumption was that we have other scenarios when we are in=20
>> realmode (specifically with MSR_RI unset) where we won't be able to=20
>> recover from a trap, during function tracing (*). I did a set of=20
>> experiments yesterday to verify that, but I was not able to uncover=20
>> any such scenarios with my brief testing. So, we seem to be=20
>> functioning just fine while tracing realmode C code, except for KVM.
>=20
> Hmm. If MSR_RI is clear then that should indicate that you can't recover
> from an interrupt, typically because you'd lose state in SRR0/1. So I
> would expect things to go badly in that case.

Yes, so it looks like we aren't calling into any C code (that isn't=20
already annotated with 'notrace') with MSR_RI unset. At least, with the=20
usual interrupt handling on powernv. I tested this by putting a 'trap'=20
in the function tracer callback after the recursion test. This forces a=20
trap for each function that we trace non-recursively.

There may be other paths where we do so, but it isn't as pervasive as I=20
previously thought. So, we should be able to exclude those paths using=20
the paca field, as and when we find them.

- Naveen

=

^ permalink raw reply

* [RFC PATCH 0/6] arm64: untag user pointers passed to the kernel
From: Andrey Konovalov @ 2018-03-09 14:01 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Mark Rutland, Robin Murphy, Al Viro,
	Philippe Ombredanne, Greg Kroah-Hartman, Thomas Gleixner,
	Kate Stewart, Andrew Morton, Kirill A . Shutemov, Ingo Molnar,
	Aneesh Kumar K . V, Minchan Kim, Michal Hocko, Shaohua Li,
	Andrea Arcangeli, Anshuman Khandual, Mike Rapoport,
	Vlastimil Babka, Naoya Horiguchi, Shakeel Butt, Joonsoo Kim,
	Hugh Dickins, Mel Gorman, Jérôme Glisse, Mike Kravetz,
	Zi Yan, linux-arm-kernel, linux-kernel, linux-mm,
	Richard Henderson, Ivan Kokshaysky, Matt Turner, Vineet Gupta,
	Russell King, Mark Salter, Aurelien Jacquiot, Mikael Starvik,
	Jesper Nilsson, Tony Luck, Fenghua Yu, Geert Uytterhoeven,
	James Hogan, Michal Simek, Ralf Baechle, David Howells,
	Ley Foon Tan, Jonas Bonn, Stefan Kristiansson, Stafford Horne,
	James E . J . Bottomley, Helge Deller, Benjamin Herrenschmidt,
	Paul Mackerras, Michael Ellerman, Palmer Dabbelt, Albert Ou,
	Chen Liqin, Lennox Wu, Yoshinori Sato, Rich Felker,
	David S . Miller, Ingo Molnar, x86, Chris Zankel, Max Filippov,
	Arnd Bergmann, linux-alpha, linux-snps-arc, adi-buildroot-devel,
	linux-c6x-dev, linux-cris-kernel, linux-ia64, linux-m68k,
	linux-metag, linux-mips, linux-am33-list, nios2-dev, openrisc,
	linux-parisc, linuxppc-dev, linux-riscv, linux-sh, sparclinux,
	linux-xtensa, linux-arch
  Cc: Dmitry Vyukov, Kostya Serebryany, Evgeniy Stepanov, Lee Smith,
	Ramana Radhakrishnan, Jacob Bramley, Ruben Ayrapetyan,
	Andrey Konovalov

arm64 has a feature called Top Byte Ignore, which allows to embed pointer
tags into the top byte of each pointer. Userspace programs (such as
HWASan, a memory debugging tool [1]) might use this feature and pass
tagged user pointers to the kernel through syscalls or other interfaces.

This patch makes a few of the kernel interfaces accept tagged user
pointers. The kernel is already able to handle user faults with tagged
pointers and has the untagged_addr macro, which this patchset reuses.

We're not trying to cover all possible ways the kernel accepts user
pointers in one patchset, so this one should be considered as a start.
It would be nice to learn about the interfaces that I missed though.

Sending this as an RFC, as I'm not sure if this should be committed as is,
and would like to receive some feedback.

Thanks!

[1] http://clang.llvm.org/docs/HardwareAssistedAddressSanitizerDesign.html

Andrey Konovalov (6):
  arm64: add type casts to untagged_addr macro
  arm64: untag user addresses in copy_from_user and others
  mm, arm64: untag user addresses in memory syscalls
  mm, arm64: untag user addresses in mm/gup.c
  lib, arm64: untag addrs passed to strncpy_from_user and strnlen_user
  arch: add untagged_addr definition for other arches

 arch/alpha/include/asm/uaccess.h      |  2 ++
 arch/arc/include/asm/uaccess.h        |  1 +
 arch/arm/include/asm/uaccess.h        |  2 ++
 arch/arm64/include/asm/uaccess.h      |  9 +++++++--
 arch/blackfin/include/asm/uaccess.h   |  2 ++
 arch/c6x/include/asm/uaccess.h        |  2 ++
 arch/cris/include/asm/uaccess.h       |  2 ++
 arch/frv/include/asm/uaccess.h        |  2 ++
 arch/ia64/include/asm/uaccess.h       |  2 ++
 arch/m32r/include/asm/uaccess.h       |  2 ++
 arch/m68k/include/asm/uaccess.h       |  2 ++
 arch/metag/include/asm/uaccess.h      |  2 ++
 arch/microblaze/include/asm/uaccess.h |  2 ++
 arch/mips/include/asm/uaccess.h       |  2 ++
 arch/mn10300/include/asm/uaccess.h    |  2 ++
 arch/nios2/include/asm/uaccess.h      |  2 ++
 arch/openrisc/include/asm/uaccess.h   |  2 ++
 arch/parisc/include/asm/uaccess.h     |  2 ++
 arch/powerpc/include/asm/uaccess.h    |  2 ++
 arch/riscv/include/asm/uaccess.h      |  2 ++
 arch/score/include/asm/uaccess.h      |  2 ++
 arch/sh/include/asm/uaccess.h         |  2 ++
 arch/sparc/include/asm/uaccess.h      |  2 ++
 arch/tile/include/asm/uaccess.h       |  2 ++
 arch/x86/include/asm/uaccess.h        |  2 ++
 arch/xtensa/include/asm/uaccess.h     |  2 ++
 include/asm-generic/uaccess.h         |  2 ++
 lib/strncpy_from_user.c               |  2 ++
 lib/strnlen_user.c                    |  2 ++
 mm/gup.c                              | 12 ++++++++++++
 mm/madvise.c                          |  2 ++
 mm/mempolicy.c                        |  6 ++++++
 mm/mincore.c                          |  2 ++
 mm/mlock.c                            |  5 +++++
 mm/mmap.c                             |  9 +++++++++
 mm/mprotect.c                         |  2 ++
 mm/mremap.c                           |  2 ++
 mm/msync.c                            |  3 +++
 38 files changed, 105 insertions(+), 2 deletions(-)

-- 
2.16.2.395.g2e18187dfd-goog

^ permalink raw reply

* [RFC PATCH 1/6] arm64: add type casts to untagged_addr macro
From: Andrey Konovalov @ 2018-03-09 14:01 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Mark Rutland, Robin Murphy, Al Viro,
	Philippe Ombredanne, Greg Kroah-Hartman, Thomas Gleixner,
	Kate Stewart, Andrew Morton, Kirill A . Shutemov, Ingo Molnar,
	Aneesh Kumar K . V, Minchan Kim, Michal Hocko, Shaohua Li,
	Andrea Arcangeli, Anshuman Khandual, Mike Rapoport,
	Vlastimil Babka, Naoya Horiguchi, Shakeel Butt, Joonsoo Kim,
	Hugh Dickins, Mel Gorman, Jérôme Glisse, Mike Kravetz,
	Zi Yan, linux-arm-kernel, linux-kernel, linux-mm,
	Richard Henderson, Ivan Kokshaysky, Matt Turner, Vineet Gupta,
	Russell King, Mark Salter, Aurelien Jacquiot, Mikael Starvik,
	Jesper Nilsson, Tony Luck, Fenghua Yu, Geert Uytterhoeven,
	James Hogan, Michal Simek, Ralf Baechle, David Howells,
	Ley Foon Tan, Jonas Bonn, Stefan Kristiansson, Stafford Horne,
	James E . J . Bottomley, Helge Deller, Benjamin Herrenschmidt,
	Paul Mackerras, Michael Ellerman, Palmer Dabbelt, Albert Ou,
	Chen Liqin, Lennox Wu, Yoshinori Sato, Rich Felker,
	David S . Miller, Ingo Molnar, x86, Chris Zankel, Max Filippov,
	Arnd Bergmann, linux-alpha, linux-snps-arc, adi-buildroot-devel,
	linux-c6x-dev, linux-cris-kernel, linux-ia64, linux-m68k,
	linux-metag, linux-mips, linux-am33-list, nios2-dev, openrisc,
	linux-parisc, linuxppc-dev, linux-riscv, linux-sh, sparclinux,
	linux-xtensa, linux-arch
  Cc: Dmitry Vyukov, Kostya Serebryany, Evgeniy Stepanov, Lee Smith,
	Ramana Radhakrishnan, Jacob Bramley, Ruben Ayrapetyan,
	Andrey Konovalov
In-Reply-To: <cover.1520600533.git.andreyknvl@google.com>

This patch makes the untagged_addr macro accept all kinds of address types
(void *, unsigned long, etc.) and allows not to specify type casts in each
place where it is used. This is done by using __typeof__.

Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
 arch/arm64/include/asm/uaccess.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
index e66b0fca99c2..2d6451cbaa86 100644
--- a/arch/arm64/include/asm/uaccess.h
+++ b/arch/arm64/include/asm/uaccess.h
@@ -102,7 +102,8 @@ static inline unsigned long __range_ok(const void __user *addr, unsigned long si
  * up with a tagged userland pointer. Clear the tag to get a sane pointer to
  * pass on to access_ok(), for instance.
  */
-#define untagged_addr(addr)		sign_extend64(addr, 55)
+#define untagged_addr(addr)		\
+	((__typeof__(addr))sign_extend64((__u64)(addr), 55))
 
 #define access_ok(type, addr, size)	__range_ok(addr, size)
 #define user_addr_max			get_fs
-- 
2.16.2.395.g2e18187dfd-goog

^ permalink raw reply related

* [RFC PATCH 2/6] arm64: untag user addresses in copy_from_user and others
From: Andrey Konovalov @ 2018-03-09 14:02 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Mark Rutland, Robin Murphy, Al Viro,
	Philippe Ombredanne, Greg Kroah-Hartman, Thomas Gleixner,
	Kate Stewart, Andrew Morton, Kirill A . Shutemov, Ingo Molnar,
	Aneesh Kumar K . V, Minchan Kim, Michal Hocko, Shaohua Li,
	Andrea Arcangeli, Anshuman Khandual, Mike Rapoport,
	Vlastimil Babka, Naoya Horiguchi, Shakeel Butt, Joonsoo Kim,
	Hugh Dickins, Mel Gorman, Jérôme Glisse, Mike Kravetz,
	Zi Yan, linux-arm-kernel, linux-kernel, linux-mm,
	Richard Henderson, Ivan Kokshaysky, Matt Turner, Vineet Gupta,
	Russell King, Mark Salter, Aurelien Jacquiot, Mikael Starvik,
	Jesper Nilsson, Tony Luck, Fenghua Yu, Geert Uytterhoeven,
	James Hogan, Michal Simek, Ralf Baechle, David Howells,
	Ley Foon Tan, Jonas Bonn, Stefan Kristiansson, Stafford Horne,
	James E . J . Bottomley, Helge Deller, Benjamin Herrenschmidt,
	Paul Mackerras, Michael Ellerman, Palmer Dabbelt, Albert Ou,
	Chen Liqin, Lennox Wu, Yoshinori Sato, Rich Felker,
	David S . Miller, Ingo Molnar, x86, Chris Zankel, Max Filippov,
	Arnd Bergmann, linux-alpha, linux-snps-arc, adi-buildroot-devel,
	linux-c6x-dev, linux-cris-kernel, linux-ia64, linux-m68k,
	linux-metag, linux-mips, linux-am33-list, nios2-dev, openrisc,
	linux-parisc, linuxppc-dev, linux-riscv, linux-sh, sparclinux,
	linux-xtensa, linux-arch
  Cc: Dmitry Vyukov, Kostya Serebryany, Evgeniy Stepanov, Lee Smith,
	Ramana Radhakrishnan, Jacob Bramley, Ruben Ayrapetyan,
	Andrey Konovalov
In-Reply-To: <cover.1520600533.git.andreyknvl@google.com>

copy_from_user (and a few other similar functions) are used to copy data
from user memory into the kernel memory or vice versa. Since a user can
provided a tagged pointer to one of the syscalls that use copy_from_user,
we need to correctly handle such pointers.

Do this by untagging user pointers in access_ok and in __uaccess_mask_ptr.

Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
 arch/arm64/include/asm/uaccess.h | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
index 2d6451cbaa86..24a221678fe3 100644
--- a/arch/arm64/include/asm/uaccess.h
+++ b/arch/arm64/include/asm/uaccess.h
@@ -105,7 +105,8 @@ static inline unsigned long __range_ok(const void __user *addr, unsigned long si
 #define untagged_addr(addr)		\
 	((__typeof__(addr))sign_extend64((__u64)(addr), 55))
 
-#define access_ok(type, addr, size)	__range_ok(addr, size)
+#define access_ok(type, addr, size)	\
+	__range_ok(untagged_addr(addr), size)
 #define user_addr_max			get_fs
 
 #define _ASM_EXTABLE(from, to)						\
@@ -238,12 +239,15 @@ static inline void uaccess_enable_not_uao(void)
 /*
  * Sanitise a uaccess pointer such that it becomes NULL if above the
  * current addr_limit.
+ * Also untag user pointers that have the top byte tag set.
  */
 #define uaccess_mask_ptr(ptr) (__typeof__(ptr))__uaccess_mask_ptr(ptr)
 static inline void __user *__uaccess_mask_ptr(const void __user *ptr)
 {
 	void __user *safe_ptr;
 
+	ptr = untagged_addr(ptr);
+
 	asm volatile(
 	"	bics	xzr, %1, %2\n"
 	"	csel	%0, %1, xzr, eq\n"
-- 
2.16.2.395.g2e18187dfd-goog

^ permalink raw reply related

* [RFC PATCH 3/6] mm, arm64: untag user addresses in memory syscalls
From: Andrey Konovalov @ 2018-03-09 14:02 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Mark Rutland, Robin Murphy, Al Viro,
	Philippe Ombredanne, Greg Kroah-Hartman, Thomas Gleixner,
	Kate Stewart, Andrew Morton, Kirill A . Shutemov, Ingo Molnar,
	Aneesh Kumar K . V, Minchan Kim, Michal Hocko, Shaohua Li,
	Andrea Arcangeli, Anshuman Khandual, Mike Rapoport,
	Vlastimil Babka, Naoya Horiguchi, Shakeel Butt, Joonsoo Kim,
	Hugh Dickins, Mel Gorman, Jérôme Glisse, Mike Kravetz,
	Zi Yan, linux-arm-kernel, linux-kernel, linux-mm,
	Richard Henderson, Ivan Kokshaysky, Matt Turner, Vineet Gupta,
	Russell King, Mark Salter, Aurelien Jacquiot, Mikael Starvik,
	Jesper Nilsson, Tony Luck, Fenghua Yu, Geert Uytterhoeven,
	James Hogan, Michal Simek, Ralf Baechle, David Howells,
	Ley Foon Tan, Jonas Bonn, Stefan Kristiansson, Stafford Horne,
	James E . J . Bottomley, Helge Deller, Benjamin Herrenschmidt,
	Paul Mackerras, Michael Ellerman, Palmer Dabbelt, Albert Ou,
	Chen Liqin, Lennox Wu, Yoshinori Sato, Rich Felker,
	David S . Miller, Ingo Molnar, x86, Chris Zankel, Max Filippov,
	Arnd Bergmann, linux-alpha, linux-snps-arc, adi-buildroot-devel,
	linux-c6x-dev, linux-cris-kernel, linux-ia64, linux-m68k,
	linux-metag, linux-mips, linux-am33-list, nios2-dev, openrisc,
	linux-parisc, linuxppc-dev, linux-riscv, linux-sh, sparclinux,
	linux-xtensa, linux-arch
  Cc: Dmitry Vyukov, Kostya Serebryany, Evgeniy Stepanov, Lee Smith,
	Ramana Radhakrishnan, Jacob Bramley, Ruben Ayrapetyan,
	Andrey Konovalov
In-Reply-To: <cover.1520600533.git.andreyknvl@google.com>

Memory subsystem syscalls accept user addresses as arguments, but don't use
copy_from_user and other similar functions, so we need to handle this case
separately.

Untag user pointers passed to madvise, mbind, get_mempolicy, mincore,
mlock, mlock2, brk, mmap_pgoff, old_mmap, munmap, remap_file_pages,
mprotect, pkey_mprotect, mremap and msync.

Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
 mm/madvise.c   | 2 ++
 mm/mempolicy.c | 6 ++++++
 mm/mincore.c   | 2 ++
 mm/mlock.c     | 5 +++++
 mm/mmap.c      | 9 +++++++++
 mm/mprotect.c  | 2 ++
 mm/mremap.c    | 2 ++
 mm/msync.c     | 3 +++
 8 files changed, 31 insertions(+)

diff --git a/mm/madvise.c b/mm/madvise.c
index 4d3c922ea1a1..909d6ba09031 100644
--- a/mm/madvise.c
+++ b/mm/madvise.c
@@ -798,6 +798,8 @@ SYSCALL_DEFINE3(madvise, unsigned long, start, size_t, len_in, int, behavior)
 	size_t len;
 	struct blk_plug plug;
 
+	start = untagged_addr(start);
+
 	if (!madvise_behavior_valid(behavior))
 		return error;
 
diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index d879f1d8a44a..79d33a570c60 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -1344,6 +1344,8 @@ SYSCALL_DEFINE6(mbind, unsigned long, start, unsigned long, len,
 	int err;
 	unsigned short mode_flags;
 
+	start = untagged_addr(start);
+
 	mode_flags = mode & MPOL_MODE_FLAGS;
 	mode &= ~MPOL_MODE_FLAGS;
 	if (mode >= MPOL_MAX)
@@ -1479,6 +1481,8 @@ SYSCALL_DEFINE5(get_mempolicy, int __user *, policy,
 	int uninitialized_var(pval);
 	nodemask_t nodes;
 
+	addr = untagged_addr(addr);
+
 	if (nmask != NULL && maxnode < MAX_NUMNODES)
 		return -EINVAL;
 
@@ -1557,6 +1561,8 @@ COMPAT_SYSCALL_DEFINE6(mbind, compat_ulong_t, start, compat_ulong_t, len,
 	unsigned long nr_bits, alloc_size;
 	nodemask_t bm;
 
+	start = untagged_addr(start);
+
 	nr_bits = min_t(unsigned long, maxnode-1, MAX_NUMNODES);
 	alloc_size = ALIGN(nr_bits, BITS_PER_LONG) / 8;
 
diff --git a/mm/mincore.c b/mm/mincore.c
index fc37afe226e6..b59cf8fa3050 100644
--- a/mm/mincore.c
+++ b/mm/mincore.c
@@ -228,6 +228,8 @@ SYSCALL_DEFINE3(mincore, unsigned long, start, size_t, len,
 	unsigned long pages;
 	unsigned char *tmp;
 
+	start = untagged_addr(start);
+
 	/* Check the start address: needs to be page-aligned.. */
 	if (start & ~PAGE_MASK)
 		return -EINVAL;
diff --git a/mm/mlock.c b/mm/mlock.c
index 74e5a6547c3d..2f456a458cac 100644
--- a/mm/mlock.c
+++ b/mm/mlock.c
@@ -714,6 +714,7 @@ static __must_check int do_mlock(unsigned long start, size_t len, vm_flags_t fla
 
 SYSCALL_DEFINE2(mlock, unsigned long, start, size_t, len)
 {
+	start = untagged_addr(start);
 	return do_mlock(start, len, VM_LOCKED);
 }
 
@@ -721,6 +722,8 @@ SYSCALL_DEFINE3(mlock2, unsigned long, start, size_t, len, int, flags)
 {
 	vm_flags_t vm_flags = VM_LOCKED;
 
+	start = untagged_addr(start);
+
 	if (flags & ~MLOCK_ONFAULT)
 		return -EINVAL;
 
@@ -734,6 +737,8 @@ SYSCALL_DEFINE2(munlock, unsigned long, start, size_t, len)
 {
 	int ret;
 
+	start = untagged_addr(start);
+
 	len = PAGE_ALIGN(len + (offset_in_page(start)));
 	start &= PAGE_MASK;
 
diff --git a/mm/mmap.c b/mm/mmap.c
index 9efdc021ad22..b63362c45cde 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -189,6 +189,8 @@ SYSCALL_DEFINE1(brk, unsigned long, brk)
 	bool populate;
 	LIST_HEAD(uf);
 
+	brk = untagged_addr(brk);
+
 	if (down_write_killable(&mm->mmap_sem))
 		return -EINTR;
 
@@ -1495,6 +1497,8 @@ SYSCALL_DEFINE6(mmap_pgoff, unsigned long, addr, unsigned long, len,
 	struct file *file = NULL;
 	unsigned long retval;
 
+	addr = untagged_addr(addr);
+
 	if (!(flags & MAP_ANONYMOUS)) {
 		audit_mmap_fd(fd, flags);
 		file = fget(fd);
@@ -1556,6 +1560,8 @@ SYSCALL_DEFINE1(old_mmap, struct mmap_arg_struct __user *, arg)
 	if (offset_in_page(a.offset))
 		return -EINVAL;
 
+	a.addr = untagged_addr(a.addr);
+
 	return sys_mmap_pgoff(a.addr, a.len, a.prot, a.flags, a.fd,
 			      a.offset >> PAGE_SHIFT);
 }
@@ -2751,6 +2757,7 @@ EXPORT_SYMBOL(vm_munmap);
 
 SYSCALL_DEFINE2(munmap, unsigned long, addr, size_t, len)
 {
+	addr = untagged_addr(addr);
 	profile_munmap(addr);
 	return vm_munmap(addr, len);
 }
@@ -2769,6 +2776,8 @@ SYSCALL_DEFINE5(remap_file_pages, unsigned long, start, unsigned long, size,
 	unsigned long ret = -EINVAL;
 	struct file *file;
 
+	start = untagged_addr(start);
+
 	pr_warn_once("%s (%d) uses deprecated remap_file_pages() syscall. See Documentation/vm/remap_file_pages.txt.\n",
 		     current->comm, current->pid);
 
diff --git a/mm/mprotect.c b/mm/mprotect.c
index e3309fcf586b..73d2a6befcf9 100644
--- a/mm/mprotect.c
+++ b/mm/mprotect.c
@@ -519,6 +519,7 @@ static int do_mprotect_pkey(unsigned long start, size_t len,
 SYSCALL_DEFINE3(mprotect, unsigned long, start, size_t, len,
 		unsigned long, prot)
 {
+	start = untagged_addr(start);
 	return do_mprotect_pkey(start, len, prot, -1);
 }
 
@@ -527,6 +528,7 @@ SYSCALL_DEFINE3(mprotect, unsigned long, start, size_t, len,
 SYSCALL_DEFINE4(pkey_mprotect, unsigned long, start, size_t, len,
 		unsigned long, prot, int, pkey)
 {
+	start = untagged_addr(start);
 	return do_mprotect_pkey(start, len, prot, pkey);
 }
 
diff --git a/mm/mremap.c b/mm/mremap.c
index 049470aa1e3e..e42863a135de 100644
--- a/mm/mremap.c
+++ b/mm/mremap.c
@@ -533,6 +533,8 @@ SYSCALL_DEFINE5(mremap, unsigned long, addr, unsigned long, old_len,
 	LIST_HEAD(uf_unmap_early);
 	LIST_HEAD(uf_unmap);
 
+	addr = untagged_addr(addr);
+
 	if (flags & ~(MREMAP_FIXED | MREMAP_MAYMOVE))
 		return ret;
 
diff --git a/mm/msync.c b/mm/msync.c
index ef30a429623a..03a977558f9f 100644
--- a/mm/msync.c
+++ b/mm/msync.c
@@ -37,12 +37,15 @@ SYSCALL_DEFINE3(msync, unsigned long, start, size_t, len, int, flags)
 	int unmapped_error = 0;
 	int error = -EINVAL;
 
+	start = untagged_addr(start);
+
 	if (flags & ~(MS_ASYNC | MS_INVALIDATE | MS_SYNC))
 		goto out;
 	if (offset_in_page(start))
 		goto out;
 	if ((flags & MS_ASYNC) && (flags & MS_SYNC))
 		goto out;
+
 	error = -ENOMEM;
 	len = (len + ~PAGE_MASK) & PAGE_MASK;
 	end = start + len;
-- 
2.16.2.395.g2e18187dfd-goog

^ permalink raw reply related

* [RFC PATCH 4/6] mm, arm64: untag user addresses in mm/gup.c
From: Andrey Konovalov @ 2018-03-09 14:02 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Mark Rutland, Robin Murphy, Al Viro,
	Philippe Ombredanne, Greg Kroah-Hartman, Thomas Gleixner,
	Kate Stewart, Andrew Morton, Kirill A . Shutemov, Ingo Molnar,
	Aneesh Kumar K . V, Minchan Kim, Michal Hocko, Shaohua Li,
	Andrea Arcangeli, Anshuman Khandual, Mike Rapoport,
	Vlastimil Babka, Naoya Horiguchi, Shakeel Butt, Joonsoo Kim,
	Hugh Dickins, Mel Gorman, Jérôme Glisse, Mike Kravetz,
	Zi Yan, linux-arm-kernel, linux-kernel, linux-mm,
	Richard Henderson, Ivan Kokshaysky, Matt Turner, Vineet Gupta,
	Russell King, Mark Salter, Aurelien Jacquiot, Mikael Starvik,
	Jesper Nilsson, Tony Luck, Fenghua Yu, Geert Uytterhoeven,
	James Hogan, Michal Simek, Ralf Baechle, David Howells,
	Ley Foon Tan, Jonas Bonn, Stefan Kristiansson, Stafford Horne,
	James E . J . Bottomley, Helge Deller, Benjamin Herrenschmidt,
	Paul Mackerras, Michael Ellerman, Palmer Dabbelt, Albert Ou,
	Chen Liqin, Lennox Wu, Yoshinori Sato, Rich Felker,
	David S . Miller, Ingo Molnar, x86, Chris Zankel, Max Filippov,
	Arnd Bergmann, linux-alpha, linux-snps-arc, adi-buildroot-devel,
	linux-c6x-dev, linux-cris-kernel, linux-ia64, linux-m68k,
	linux-metag, linux-mips, linux-am33-list, nios2-dev, openrisc,
	linux-parisc, linuxppc-dev, linux-riscv, linux-sh, sparclinux,
	linux-xtensa, linux-arch
  Cc: Dmitry Vyukov, Kostya Serebryany, Evgeniy Stepanov, Lee Smith,
	Ramana Radhakrishnan, Jacob Bramley, Ruben Ayrapetyan,
	Andrey Konovalov
In-Reply-To: <cover.1520600533.git.andreyknvl@google.com>

mm/gup.c provides a kernel interface that accepts user addresses and
manipulates user pages directly (for example get_user_pages, that is used
by the futex syscall). Here we also need to handle the case of tagged user
pointers.

Untag addresses passed to this interface.

Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
 mm/gup.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/mm/gup.c b/mm/gup.c
index 1b46e6e74881..4d820c4792d7 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -386,6 +386,8 @@ struct page *follow_page_mask(struct vm_area_struct *vma,
 	struct page *page;
 	struct mm_struct *mm = vma->vm_mm;
 
+	address = untagged_addr(address);
+
 	*page_mask = 0;
 
 	/* make this handle hugepd */
@@ -647,6 +649,8 @@ static long __get_user_pages(struct task_struct *tsk, struct mm_struct *mm,
 	if (!nr_pages)
 		return 0;
 
+	start = untagged_addr(start);
+
 	VM_BUG_ON(!!pages != !!(gup_flags & FOLL_GET));
 
 	/*
@@ -801,6 +805,8 @@ int fixup_user_fault(struct task_struct *tsk, struct mm_struct *mm,
 	struct vm_area_struct *vma;
 	int ret, major = 0;
 
+	address = untagged_addr(address);
+
 	if (unlocked)
 		fault_flags |= FAULT_FLAG_ALLOW_RETRY;
 
@@ -854,6 +860,8 @@ static __always_inline long __get_user_pages_locked(struct task_struct *tsk,
 	long ret, pages_done;
 	bool lock_dropped;
 
+	start = untagged_addr(start);
+
 	if (locked) {
 		/* if VM_FAULT_RETRY can be returned, vmas become invalid */
 		BUG_ON(vmas);
@@ -1746,6 +1754,8 @@ int __get_user_pages_fast(unsigned long start, int nr_pages, int write,
 	unsigned long flags;
 	int nr = 0;
 
+	start = untagged_addr(start);
+
 	start &= PAGE_MASK;
 	addr = start;
 	len = (unsigned long) nr_pages << PAGE_SHIFT;
@@ -1798,6 +1808,8 @@ int get_user_pages_fast(unsigned long start, int nr_pages, int write,
 	unsigned long addr, len, end;
 	int nr = 0, ret = 0;
 
+	start = untagged_addr(start);
+
 	start &= PAGE_MASK;
 	addr = start;
 	len = (unsigned long) nr_pages << PAGE_SHIFT;
-- 
2.16.2.395.g2e18187dfd-goog

^ permalink raw reply related

* [RFC PATCH 5/6] lib, arm64: untag addrs passed to strncpy_from_user and strnlen_user
From: Andrey Konovalov @ 2018-03-09 14:02 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Mark Rutland, Robin Murphy, Al Viro,
	Philippe Ombredanne, Greg Kroah-Hartman, Thomas Gleixner,
	Kate Stewart, Andrew Morton, Kirill A . Shutemov, Ingo Molnar,
	Aneesh Kumar K . V, Minchan Kim, Michal Hocko, Shaohua Li,
	Andrea Arcangeli, Anshuman Khandual, Mike Rapoport,
	Vlastimil Babka, Naoya Horiguchi, Shakeel Butt, Joonsoo Kim,
	Hugh Dickins, Mel Gorman, Jérôme Glisse, Mike Kravetz,
	Zi Yan, linux-arm-kernel, linux-kernel, linux-mm,
	Richard Henderson, Ivan Kokshaysky, Matt Turner, Vineet Gupta,
	Russell King, Mark Salter, Aurelien Jacquiot, Mikael Starvik,
	Jesper Nilsson, Tony Luck, Fenghua Yu, Geert Uytterhoeven,
	James Hogan, Michal Simek, Ralf Baechle, David Howells,
	Ley Foon Tan, Jonas Bonn, Stefan Kristiansson, Stafford Horne,
	James E . J . Bottomley, Helge Deller, Benjamin Herrenschmidt,
	Paul Mackerras, Michael Ellerman, Palmer Dabbelt, Albert Ou,
	Chen Liqin, Lennox Wu, Yoshinori Sato, Rich Felker,
	David S . Miller, Ingo Molnar, x86, Chris Zankel, Max Filippov,
	Arnd Bergmann, linux-alpha, linux-snps-arc, adi-buildroot-devel,
	linux-c6x-dev, linux-cris-kernel, linux-ia64, linux-m68k,
	linux-metag, linux-mips, linux-am33-list, nios2-dev, openrisc,
	linux-parisc, linuxppc-dev, linux-riscv, linux-sh, sparclinux,
	linux-xtensa, linux-arch
  Cc: Dmitry Vyukov, Kostya Serebryany, Evgeniy Stepanov, Lee Smith,
	Ramana Radhakrishnan, Jacob Bramley, Ruben Ayrapetyan,
	Andrey Konovalov
In-Reply-To: <cover.1520600533.git.andreyknvl@google.com>

strncpy_from_user and strnlen_user accept user addresses as arguments, and
do not go through the same path as copy_from_user and others, so here we
need to separately handle the case of tagged user addresses as well.

Untag user pointers passed to these functions.

Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
 lib/strncpy_from_user.c | 2 ++
 lib/strnlen_user.c      | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/lib/strncpy_from_user.c b/lib/strncpy_from_user.c
index b53e1b5d80f4..97467cd2bc59 100644
--- a/lib/strncpy_from_user.c
+++ b/lib/strncpy_from_user.c
@@ -106,6 +106,8 @@ long strncpy_from_user(char *dst, const char __user *src, long count)
 	if (unlikely(count <= 0))
 		return 0;
 
+	src = untagged_addr(src);
+
 	max_addr = user_addr_max();
 	src_addr = (unsigned long)src;
 	if (likely(src_addr < max_addr)) {
diff --git a/lib/strnlen_user.c b/lib/strnlen_user.c
index 60d0bbda8f5e..8b5f56466e00 100644
--- a/lib/strnlen_user.c
+++ b/lib/strnlen_user.c
@@ -108,6 +108,8 @@ long strnlen_user(const char __user *str, long count)
 	if (unlikely(count <= 0))
 		return 0;
 
+	str = untagged_addr(str);
+
 	max_addr = user_addr_max();
 	src_addr = (unsigned long)str;
 	if (likely(src_addr < max_addr)) {
-- 
2.16.2.395.g2e18187dfd-goog

^ permalink raw reply related

* [RFC PATCH 6/6] arch: add untagged_addr definition for other arches
From: Andrey Konovalov @ 2018-03-09 14:02 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Mark Rutland, Robin Murphy, Al Viro,
	Philippe Ombredanne, Greg Kroah-Hartman, Thomas Gleixner,
	Kate Stewart, Andrew Morton, Kirill A . Shutemov, Ingo Molnar,
	Aneesh Kumar K . V, Minchan Kim, Michal Hocko, Shaohua Li,
	Andrea Arcangeli, Anshuman Khandual, Mike Rapoport,
	Vlastimil Babka, Naoya Horiguchi, Shakeel Butt, Joonsoo Kim,
	Hugh Dickins, Mel Gorman, Jérôme Glisse, Mike Kravetz,
	Zi Yan, linux-arm-kernel, linux-kernel, linux-mm,
	Richard Henderson, Ivan Kokshaysky, Matt Turner, Vineet Gupta,
	Russell King, Mark Salter, Aurelien Jacquiot, Mikael Starvik,
	Jesper Nilsson, Tony Luck, Fenghua Yu, Geert Uytterhoeven,
	James Hogan, Michal Simek, Ralf Baechle, David Howells,
	Ley Foon Tan, Jonas Bonn, Stefan Kristiansson, Stafford Horne,
	James E . J . Bottomley, Helge Deller, Benjamin Herrenschmidt,
	Paul Mackerras, Michael Ellerman, Palmer Dabbelt, Albert Ou,
	Chen Liqin, Lennox Wu, Yoshinori Sato, Rich Felker,
	David S . Miller, Ingo Molnar, x86, Chris Zankel, Max Filippov,
	Arnd Bergmann, linux-alpha, linux-snps-arc, adi-buildroot-devel,
	linux-c6x-dev, linux-cris-kernel, linux-ia64, linux-m68k,
	linux-metag, linux-mips, linux-am33-list, nios2-dev, openrisc,
	linux-parisc, linuxppc-dev, linux-riscv, linux-sh, sparclinux,
	linux-xtensa, linux-arch
  Cc: Dmitry Vyukov, Kostya Serebryany, Evgeniy Stepanov, Lee Smith,
	Ramana Radhakrishnan, Jacob Bramley, Ruben Ayrapetyan,
	Andrey Konovalov
In-Reply-To: <cover.1520600533.git.andreyknvl@google.com>

To allow arm64 syscalls accept tagged pointers from userspace, we must
untag them when they are passed to the kernel. Since untagging is done in
generic parts of the kernel (like the mm subsystem), the untagged_addr
macro should be defined for all architectures.

Define it as a noop for all other architectures besides arm64.

Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
 arch/alpha/include/asm/uaccess.h      | 2 ++
 arch/arc/include/asm/uaccess.h        | 1 +
 arch/arm/include/asm/uaccess.h        | 2 ++
 arch/blackfin/include/asm/uaccess.h   | 2 ++
 arch/c6x/include/asm/uaccess.h        | 2 ++
 arch/cris/include/asm/uaccess.h       | 2 ++
 arch/frv/include/asm/uaccess.h        | 2 ++
 arch/ia64/include/asm/uaccess.h       | 2 ++
 arch/m32r/include/asm/uaccess.h       | 2 ++
 arch/m68k/include/asm/uaccess.h       | 2 ++
 arch/metag/include/asm/uaccess.h      | 2 ++
 arch/microblaze/include/asm/uaccess.h | 2 ++
 arch/mips/include/asm/uaccess.h       | 2 ++
 arch/mn10300/include/asm/uaccess.h    | 2 ++
 arch/nios2/include/asm/uaccess.h      | 2 ++
 arch/openrisc/include/asm/uaccess.h   | 2 ++
 arch/parisc/include/asm/uaccess.h     | 2 ++
 arch/powerpc/include/asm/uaccess.h    | 2 ++
 arch/riscv/include/asm/uaccess.h      | 2 ++
 arch/score/include/asm/uaccess.h      | 2 ++
 arch/sh/include/asm/uaccess.h         | 2 ++
 arch/sparc/include/asm/uaccess.h      | 2 ++
 arch/tile/include/asm/uaccess.h       | 2 ++
 arch/x86/include/asm/uaccess.h        | 2 ++
 arch/xtensa/include/asm/uaccess.h     | 2 ++
 include/asm-generic/uaccess.h         | 2 ++
 26 files changed, 51 insertions(+)

diff --git a/arch/alpha/include/asm/uaccess.h b/arch/alpha/include/asm/uaccess.h
index 87d8c4f0307d..09d136bb4ff5 100644
--- a/arch/alpha/include/asm/uaccess.h
+++ b/arch/alpha/include/asm/uaccess.h
@@ -2,6 +2,8 @@
 #ifndef __ALPHA_UACCESS_H
 #define __ALPHA_UACCESS_H
 
+#define untagged_addr(addr)	addr
+
 /*
  * The fs value determines whether argument validity checking should be
  * performed or not.  If get_fs() == USER_DS, checking is performed, with
diff --git a/arch/arc/include/asm/uaccess.h b/arch/arc/include/asm/uaccess.h
index c9173c02081c..2a04b7a4aada 100644
--- a/arch/arc/include/asm/uaccess.h
+++ b/arch/arc/include/asm/uaccess.h
@@ -26,6 +26,7 @@
 
 #include <linux/string.h>	/* for generic string functions */
 
+#define untagged_addr(addr)	addr
 
 #define __kernel_ok		(uaccess_kernel())
 
diff --git a/arch/arm/include/asm/uaccess.h b/arch/arm/include/asm/uaccess.h
index 0bf2347495f1..7d4f4e4021f2 100644
--- a/arch/arm/include/asm/uaccess.h
+++ b/arch/arm/include/asm/uaccess.h
@@ -19,6 +19,8 @@
 
 #include <asm/extable.h>
 
+#define untagged_addr(addr)	addr
+
 /*
  * These two functions allow hooking accesses to userspace to increase
  * system integrity by ensuring that the kernel can not inadvertantly
diff --git a/arch/blackfin/include/asm/uaccess.h b/arch/blackfin/include/asm/uaccess.h
index 45da4bcb050e..fb6bdc54e7bd 100644
--- a/arch/blackfin/include/asm/uaccess.h
+++ b/arch/blackfin/include/asm/uaccess.h
@@ -18,6 +18,8 @@
 #include <asm/segment.h>
 #include <asm/sections.h>
 
+#define untagged_addr(addr)	addr
+
 #define get_ds()        (KERNEL_DS)
 #define get_fs()        (current_thread_info()->addr_limit)
 
diff --git a/arch/c6x/include/asm/uaccess.h b/arch/c6x/include/asm/uaccess.h
index ba6756879f00..f187696cf440 100644
--- a/arch/c6x/include/asm/uaccess.h
+++ b/arch/c6x/include/asm/uaccess.h
@@ -9,6 +9,8 @@
 #ifndef _ASM_C6X_UACCESS_H
 #define _ASM_C6X_UACCESS_H
 
+#define untagged_addr(addr)	addr
+
 #include <linux/types.h>
 #include <linux/compiler.h>
 #include <linux/string.h>
diff --git a/arch/cris/include/asm/uaccess.h b/arch/cris/include/asm/uaccess.h
index 3b42ab0cae93..86d8fbd200c4 100644
--- a/arch/cris/include/asm/uaccess.h
+++ b/arch/cris/include/asm/uaccess.h
@@ -19,6 +19,8 @@
 #include <asm/processor.h>
 #include <asm/page.h>
 
+#define untagged_addr(addr)	addr
+
 /*
  * The fs value determines whether argument validity checking should be
  * performed or not.  If get_fs() == USER_DS, checking is performed, with
diff --git a/arch/frv/include/asm/uaccess.h b/arch/frv/include/asm/uaccess.h
index ff9562dc6825..be21b42bde09 100644
--- a/arch/frv/include/asm/uaccess.h
+++ b/arch/frv/include/asm/uaccess.h
@@ -12,6 +12,8 @@
 #ifndef _ASM_UACCESS_H
 #define _ASM_UACCESS_H
 
+#define untagged_addr(addr)	addr
+
 /*
  * User space memory access functions
  */
diff --git a/arch/ia64/include/asm/uaccess.h b/arch/ia64/include/asm/uaccess.h
index a74524f2d625..1c46bf1c4f73 100644
--- a/arch/ia64/include/asm/uaccess.h
+++ b/arch/ia64/include/asm/uaccess.h
@@ -42,6 +42,8 @@
 #include <asm/io.h>
 #include <asm/extable.h>
 
+#define untagged_addr(addr)	addr
+
 /*
  * For historical reasons, the following macros are grossly misnamed:
  */
diff --git a/arch/m32r/include/asm/uaccess.h b/arch/m32r/include/asm/uaccess.h
index 9d89bc3d8181..6e0fe6b215be 100644
--- a/arch/m32r/include/asm/uaccess.h
+++ b/arch/m32r/include/asm/uaccess.h
@@ -16,6 +16,8 @@
 #include <asm/setup.h>
 #include <linux/prefetch.h>
 
+#define untagged_addr(addr)	addr
+
 /*
  * The fs value determines whether argument validity checking should be
  * performed or not.  If get_fs() == USER_DS, checking is performed, with
diff --git a/arch/m68k/include/asm/uaccess.h b/arch/m68k/include/asm/uaccess.h
index e896466a41a4..02e0c5878ad5 100644
--- a/arch/m68k/include/asm/uaccess.h
+++ b/arch/m68k/include/asm/uaccess.h
@@ -5,3 +5,5 @@
 #include <asm/uaccess_mm.h>
 #endif
 #include <asm/extable.h>
+
+#define untagged_addr(addr)	addr
diff --git a/arch/metag/include/asm/uaccess.h b/arch/metag/include/asm/uaccess.h
index a5311eb36e32..1b2f0478868a 100644
--- a/arch/metag/include/asm/uaccess.h
+++ b/arch/metag/include/asm/uaccess.h
@@ -14,6 +14,8 @@
  * For historical reasons, these macros are grossly misnamed.
  */
 
+#define untagged_addr(addr)	addr
+
 #define MAKE_MM_SEG(s)  ((mm_segment_t) { (s) })
 
 #define KERNEL_DS       MAKE_MM_SEG(0xFFFFFFFF)
diff --git a/arch/microblaze/include/asm/uaccess.h b/arch/microblaze/include/asm/uaccess.h
index 81f16aadbf9e..a66bc26660c3 100644
--- a/arch/microblaze/include/asm/uaccess.h
+++ b/arch/microblaze/include/asm/uaccess.h
@@ -20,6 +20,8 @@
 #include <asm/extable.h>
 #include <linux/string.h>
 
+#define untagged_addr(addr)	addr
+
 /*
  * On Microblaze the fs value is actually the top of the corresponding
  * address space.
diff --git a/arch/mips/include/asm/uaccess.h b/arch/mips/include/asm/uaccess.h
index b71306947290..2db7606c388b 100644
--- a/arch/mips/include/asm/uaccess.h
+++ b/arch/mips/include/asm/uaccess.h
@@ -16,6 +16,8 @@
 #include <asm/asm-eva.h>
 #include <asm/extable.h>
 
+#define untagged_addr(addr)	addr
+
 /*
  * The fs value determines whether argument validity checking should be
  * performed or not.  If get_fs() == USER_DS, checking is performed, with
diff --git a/arch/mn10300/include/asm/uaccess.h b/arch/mn10300/include/asm/uaccess.h
index 5af468fd1359..6604699b34b6 100644
--- a/arch/mn10300/include/asm/uaccess.h
+++ b/arch/mn10300/include/asm/uaccess.h
@@ -17,6 +17,8 @@
 #include <linux/kernel.h>
 #include <asm/page.h>
 
+#define untagged_addr(addr)	addr
+
 /*
  * The fs value determines whether argument validity checking should be
  * performed or not.  If get_fs() == USER_DS, checking is performed, with
diff --git a/arch/nios2/include/asm/uaccess.h b/arch/nios2/include/asm/uaccess.h
index dfa3c7cb30b4..36152a7302a8 100644
--- a/arch/nios2/include/asm/uaccess.h
+++ b/arch/nios2/include/asm/uaccess.h
@@ -19,6 +19,8 @@
 
 #include <asm/extable.h>
 
+#define untagged_addr(addr)	addr
+
 /*
  * Segment stuff
  */
diff --git a/arch/openrisc/include/asm/uaccess.h b/arch/openrisc/include/asm/uaccess.h
index bbf5c79cce7a..5b43d13ab363 100644
--- a/arch/openrisc/include/asm/uaccess.h
+++ b/arch/openrisc/include/asm/uaccess.h
@@ -27,6 +27,8 @@
 #include <asm/page.h>
 #include <asm/extable.h>
 
+#define untagged_addr(addr)	addr
+
 /*
  * The fs value determines whether argument validity checking should be
  * performed or not.  If get_fs() == USER_DS, checking is performed, with
diff --git a/arch/parisc/include/asm/uaccess.h b/arch/parisc/include/asm/uaccess.h
index ea70e36ce6af..b0f3cd529c8d 100644
--- a/arch/parisc/include/asm/uaccess.h
+++ b/arch/parisc/include/asm/uaccess.h
@@ -11,6 +11,8 @@
 #include <linux/bug.h>
 #include <linux/string.h>
 
+#define untagged_addr(addr)	addr
+
 #define KERNEL_DS	((mm_segment_t){0})
 #define USER_DS 	((mm_segment_t){1})
 
diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
index 51bfeb8777f0..07ae1c318166 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -8,6 +8,8 @@
 #include <asm/page.h>
 #include <asm/extable.h>
 
+#define untagged_addr(addr)	addr
+
 /*
  * The fs value determines whether argument validity checking should be
  * performed or not.  If get_fs() == USER_DS, checking is performed, with
diff --git a/arch/riscv/include/asm/uaccess.h b/arch/riscv/include/asm/uaccess.h
index 14b0b22fb578..e774239aac24 100644
--- a/arch/riscv/include/asm/uaccess.h
+++ b/arch/riscv/include/asm/uaccess.h
@@ -25,6 +25,8 @@
 #include <asm/byteorder.h>
 #include <asm/asm.h>
 
+#define untagged_addr(addr)	addr
+
 #define __enable_user_access()							\
 	__asm__ __volatile__ ("csrs sstatus, %0" : : "r" (SR_SUM) : "memory")
 #define __disable_user_access()							\
diff --git a/arch/score/include/asm/uaccess.h b/arch/score/include/asm/uaccess.h
index a233f3236846..fd16c2a71091 100644
--- a/arch/score/include/asm/uaccess.h
+++ b/arch/score/include/asm/uaccess.h
@@ -5,6 +5,8 @@
 #include <linux/kernel.h>
 #include <asm/extable.h>
 
+#define untagged_addr(addr)	addr
+
 #define get_ds()		(KERNEL_DS)
 #define get_fs()		(current_thread_info()->addr_limit)
 #define segment_eq(a, b)	((a).seg == (b).seg)
diff --git a/arch/sh/include/asm/uaccess.h b/arch/sh/include/asm/uaccess.h
index 32eb56e00c11..31f3ea075190 100644
--- a/arch/sh/include/asm/uaccess.h
+++ b/arch/sh/include/asm/uaccess.h
@@ -5,6 +5,8 @@
 #include <asm/segment.h>
 #include <asm/extable.h>
 
+#define untagged_addr(addr)	addr
+
 #define __addr_ok(addr) \
 	((unsigned long __force)(addr) < current_thread_info()->addr_limit.seg)
 
diff --git a/arch/sparc/include/asm/uaccess.h b/arch/sparc/include/asm/uaccess.h
index dd85bc2c2cad..70c2f5ea09ce 100644
--- a/arch/sparc/include/asm/uaccess.h
+++ b/arch/sparc/include/asm/uaccess.h
@@ -7,6 +7,8 @@
 #include <asm/uaccess_32.h>
 #endif
 
+#define untagged_addr(addr)	addr
+
 #define user_addr_max() \
 	(uaccess_kernel() ? ~0UL : TASK_SIZE)
 
diff --git a/arch/tile/include/asm/uaccess.h b/arch/tile/include/asm/uaccess.h
index cb4fbe7e4f88..7d365b087dcb 100644
--- a/arch/tile/include/asm/uaccess.h
+++ b/arch/tile/include/asm/uaccess.h
@@ -22,6 +22,8 @@
 #include <asm/processor.h>
 #include <asm/page.h>
 
+#define untagged_addr(addr)	addr
+
 /*
  * The fs value determines whether argument validity checking should be
  * performed or not.  If get_fs() == USER_DS, checking is performed, with
diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
index aae77eb8491c..3c233fbdd32b 100644
--- a/arch/x86/include/asm/uaccess.h
+++ b/arch/x86/include/asm/uaccess.h
@@ -12,6 +12,8 @@
 #include <asm/smap.h>
 #include <asm/extable.h>
 
+#define untagged_addr(addr)	addr
+
 /*
  * The fs value determines whether argument validity checking should be
  * performed or not.  If get_fs() == USER_DS, checking is performed, with
diff --git a/arch/xtensa/include/asm/uaccess.h b/arch/xtensa/include/asm/uaccess.h
index f1158b4c629c..130e419c4d6e 100644
--- a/arch/xtensa/include/asm/uaccess.h
+++ b/arch/xtensa/include/asm/uaccess.h
@@ -20,6 +20,8 @@
 #include <asm/types.h>
 #include <asm/extable.h>
 
+#define untagged_addr(addr)	addr
+
 /*
  * The fs value determines whether argument validity checking should
  * be performed or not.  If get_fs() == USER_DS, checking is
diff --git a/include/asm-generic/uaccess.h b/include/asm-generic/uaccess.h
index 6b2e63df2739..2c46d2253dba 100644
--- a/include/asm-generic/uaccess.h
+++ b/include/asm-generic/uaccess.h
@@ -35,6 +35,8 @@ static inline void set_fs(mm_segment_t fs)
 #define segment_eq(a, b) ((a).seg == (b).seg)
 #endif
 
+#define untagged_addr(addr) addr
+
 #define access_ok(type, addr, size) __access_ok((unsigned long)(addr),(size))
 
 /*
-- 
2.16.2.395.g2e18187dfd-goog

^ permalink raw reply related

* Re: [RFC PATCH 6/6] arch: add untagged_addr definition for other arches
From: Arnd Bergmann @ 2018-03-09 14:11 UTC (permalink / raw)
  To: Andrey Konovalov
  Cc: Catalin Marinas, Will Deacon, Mark Rutland, Robin Murphy, Al Viro,
	Philippe Ombredanne, Greg Kroah-Hartman, Thomas Gleixner,
	Kate Stewart, Andrew Morton, Kirill A . Shutemov, Ingo Molnar,
	Aneesh Kumar K . V, Minchan Kim, Michal Hocko, Shaohua Li,
	Andrea Arcangeli, Anshuman Khandual, Mike Rapoport,
	Vlastimil Babka, Naoya Horiguchi, Shakeel Butt, Joonsoo Kim,
	Hugh Dickins, Mel Gorman, Jérôme Glisse, Mike Kravetz,
	Zi Yan, Linux ARM, Linux Kernel Mailing List, Linux-MM,
	Richard Henderson, Ivan Kokshaysky, Matt Turner, Vineet Gupta,
	Russell King, Mark Salter, Aurelien Jacquiot, Mikael Starvik,
	Jesper Nilsson, Tony Luck, Fenghua Yu, Geert Uytterhoeven,
	James Hogan, Michal Simek, Ralf Baechle, David Howells,
	Ley Foon Tan, Jonas Bonn, Stefan Kristiansson, Stafford Horne,
	James E . J . Bottomley, Helge Deller, Benjamin Herrenschmidt,
	Paul Mackerras, Michael Ellerman, Palmer Dabbelt, Albert Ou,
	Chen Liqin, Lennox Wu, Yoshinori Sato, Rich Felker,
	David S . Miller, Ingo Molnar, the arch/x86 maintainers,
	Chris Zankel, Max Filippov, linux-alpha,
	open list:SYNOPSYS ARC ARCHITECTURE, adi-buildroot-devel,
	linux-c6x-dev, linux-cris-kernel, linux-ia64, linux-m68k,
	open list:METAG ARCHITECTURE, open list:RALINK MIPS ARCHITECTURE,
	linux-am33-list, moderated list:NIOS2 ARCHITECTURE, openrisc,
	Parisc List, linuxppc-dev, linux-riscv, Linux-sh list, sparclinux,
	linux-xtensa, linux-arch, Dmitry Vyukov, Kostya Serebryany,
	Evgeniy Stepanov, Lee Smith, Ramana Radhakrishnan, Jacob Bramley,
	Ruben Ayrapetyan
In-Reply-To: <89b4bb181a0622d2c581699bb3814fc041078d04.1520600533.git.andreyknvl@google.com>

On Fri, Mar 9, 2018 at 3:02 PM, Andrey Konovalov <andreyknvl@google.com> wrote:
> To allow arm64 syscalls accept tagged pointers from userspace, we must
> untag them when they are passed to the kernel. Since untagging is done in
> generic parts of the kernel (like the mm subsystem), the untagged_addr
> macro should be defined for all architectures.
>
> Define it as a noop for all other architectures besides arm64.
>
> Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
> ---
>  arch/alpha/include/asm/uaccess.h      | 2 ++
>  arch/arc/include/asm/uaccess.h        | 1 +
>  arch/arm/include/asm/uaccess.h        | 2 ++
>  arch/blackfin/include/asm/uaccess.h   | 2 ++
>  arch/c6x/include/asm/uaccess.h        | 2 ++
>  arch/cris/include/asm/uaccess.h       | 2 ++
>  arch/frv/include/asm/uaccess.h        | 2 ++
>  arch/ia64/include/asm/uaccess.h       | 2 ++
>  arch/m32r/include/asm/uaccess.h       | 2 ++
>  arch/m68k/include/asm/uaccess.h       | 2 ++
>  arch/metag/include/asm/uaccess.h      | 2 ++
>  arch/microblaze/include/asm/uaccess.h | 2 ++
>  arch/mips/include/asm/uaccess.h       | 2 ++
>  arch/mn10300/include/asm/uaccess.h    | 2 ++
>  arch/nios2/include/asm/uaccess.h      | 2 ++
>  arch/openrisc/include/asm/uaccess.h   | 2 ++
>  arch/parisc/include/asm/uaccess.h     | 2 ++
>  arch/powerpc/include/asm/uaccess.h    | 2 ++
>  arch/riscv/include/asm/uaccess.h      | 2 ++
>  arch/score/include/asm/uaccess.h      | 2 ++
>  arch/sh/include/asm/uaccess.h         | 2 ++
>  arch/sparc/include/asm/uaccess.h      | 2 ++
>  arch/tile/include/asm/uaccess.h       | 2 ++
>  arch/x86/include/asm/uaccess.h        | 2 ++
>  arch/xtensa/include/asm/uaccess.h     | 2 ++
>  include/asm-generic/uaccess.h         | 2 ++
>  26 files changed, 51 insertions(+)

I have patches to remove the blackfin, cris, frv, m32r, metag, mn10300,
score, tile and unicore32 architectures from the kernel, these should be
part of linux-next in the next few days. It's not a big issue, but if you keep
patching them, this will cause a merge conflict.

It might be easier to drop them from your patch as well.

    Arnd

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox