[PATCH v13 10/20] kernel, arm64: untag user pointers in prctl_set_mm*

[Vincenzo Frascino]

Hi Andrey,

sorry for the late reply, I came back from holiday and try to catch up with the
emails.

On 4/29/19 3:23 PM, Andrey Konovalov wrote:
> On Fri, Apr 26, 2019 at 4:50 PM Catalin Marinas <catalin.marinas at arm.com> wrote:
>>
>> On Mon, Apr 01, 2019 at 06:44:34PM +0200, Andrey Konovalov wrote:
>>> On Fri, Mar 22, 2019 at 4:41 PM Catalin Marinas <catalin.marinas at arm.com> wrote:
>>>> On Wed, Mar 20, 2019 at 03:51:24PM +0100, Andrey Konovalov wrote:
>>>>> @@ -2120,13 +2135,14 @@ static int prctl_set_mm(int opt, unsigned long addr,
>>>>>       if (opt == PR_SET_MM_AUXV)
>>>>>               return prctl_set_auxv(mm, addr, arg4);
>>>>>
>>>>> -     if (addr >= TASK_SIZE || addr < mmap_min_addr)
>>>>> +     if (untagged_addr(addr) >= TASK_SIZE ||
>>>>> +                     untagged_addr(addr) < mmap_min_addr)
>>>>>               return -EINVAL;
>>>>>
>>>>>       error = -EINVAL;
>>>>>
>>>>>       down_write(&mm->mmap_sem);
>>>>> -     vma = find_vma(mm, addr);
>>>>> +     vma = find_vma(mm, untagged_addr(addr));
>>>>>
>>>>>       prctl_map.start_code    = mm->start_code;
>>>>>       prctl_map.end_code      = mm->end_code;
>>>>
>>>> Does this mean that we are left with tagged addresses for the
>>>> mm->start_code etc. values? I really don't think we should allow this,
>>>> I'm not sure what the implications are in other parts of the kernel.
>>>>
>>>> Arguably, these are not even pointer values but some address ranges. I
>>>> know we decided to relax this notion for mmap/mprotect/madvise() since
>>>> the user function prototypes take pointer as arguments but it feels like
>>>> we are overdoing it here (struct prctl_mm_map doesn't even have
>>>> pointers).
>>>>
>>>> What is the use-case for allowing tagged addresses here? Can user space
>>>> handle untagging?
>>>
>>> I don't know any use cases for this. I did it because it seems to be
>>> covered by the relaxed ABI. I'm not entirely sure what to do here,
>>> should I just drop this patch?
>>
>> If we allow tagged addresses to be passed here, we'd have to untag them
>> before they end up in the mm->start_code etc. members.
>>
>> I know we are trying to relax the ABI here w.r.t. address ranges but
>> mostly because we couldn't figure out a way to document unambiguously
>> the difference between a user pointer that may be dereferenced by the
>> kernel (tags allowed) and an address typically used for managing the
>> address space layout. Suggestions welcomed.
>>
>> I'd say just drop this patch and capture it in the ABI document.
> 
> OK, will do in v14.
> 
> Vincenzo, could you add a note about this into tour patchset?
>

Ok, I will add a note that covers this case in v3 of my document.

>>
>> --
>> Catalin

-- 
Regards,
Vincenzo