From mboxrd@z Thu Jan 1 00:00:00 1970 From: dave.hansen at intel.com (Dave Hansen) Date: Fri, 22 Feb 2019 15:07:45 -0800 Subject: [PATCH v10 04/12] mm, arm64: untag user pointers passed to memory syscalls In-Reply-To: <3875fa863b755d8cb43afa7bb0fe543e5fd05a5d.1550839937.git.andreyknvl@google.com> References: <3875fa863b755d8cb43afa7bb0fe543e5fd05a5d.1550839937.git.andreyknvl@google.com> Message-ID: <81ea4e77-90a4-4fd9-2bc8-135e0da30044@intel.com> On 2/22/19 4:53 AM, Andrey Konovalov wrote: > --- a/mm/mprotect.c > +++ b/mm/mprotect.c > @@ -578,6 +578,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); > } > > @@ -586,6 +587,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); > } This seems to have taken the approach of going as close as possible to the syscall boundary and untagging the pointer there. I guess that's OK, but it does lead to more churn than necessary. For instance, why not just do the untagging in do_mprotect_pkey()? I think that's an overall design question. I kinda asked the same thing about patching call sites vs. VMA lookup functions. From mboxrd@z Thu Jan 1 00:00:00 1970 From: dave.hansen@intel.com (Dave Hansen) Date: Fri, 22 Feb 2019 15:07:45 -0800 Subject: [PATCH v10 04/12] mm, arm64: untag user pointers passed to memory syscalls In-Reply-To: <3875fa863b755d8cb43afa7bb0fe543e5fd05a5d.1550839937.git.andreyknvl@google.com> References: <3875fa863b755d8cb43afa7bb0fe543e5fd05a5d.1550839937.git.andreyknvl@google.com> Message-ID: <81ea4e77-90a4-4fd9-2bc8-135e0da30044@intel.com> Content-Type: text/plain; charset="UTF-8" Message-ID: <20190222230745.G2VEauQ3n1q3xDY5LS8lwtFubR7lPLKQcyU86Crey90@z> On 2/22/19 4:53 AM, Andrey Konovalov wrote: > --- a/mm/mprotect.c > +++ b/mm/mprotect.c > @@ -578,6 +578,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); > } > > @@ -586,6 +587,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); > } This seems to have taken the approach of going as close as possible to the syscall boundary and untagging the pointer there. I guess that's OK, but it does lead to more churn than necessary. For instance, why not just do the untagging in do_mprotect_pkey()? I think that's an overall design question. I kinda asked the same thing about patching call sites vs. VMA lookup functions.