linux-kernel - Re: [PATCH] android: binder: use VM

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CADAEsF_L5nkioVVUpSh-=nLEyk+=kGYVQ_pueox_et69nV5ADA@mail.gmail.com>
Date:   Tue, 6 Sep 2016 10:40:07 +0800
From:   Ganesh Mahendran <opensource.ganesh@...il.com>
To:     Greg KH <gregkh@...uxfoundation.org>
Cc:     arve@...roid.com, riandrews@...roid.com,
        devel@...verdev.osuosl.org,
        linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] android: binder: use VM_ALLOC to get vm area.

Hi, Greg:

2016-09-02 3:02 GMT+08:00 Greg KH <gregkh@...uxfoundation.org>:
> On Thu, Sep 01, 2016 at 02:41:04PM +0800, Ganesh Mahendran wrote:
>> VM_IOREMAP is used to access hardware through a mechanism called
>> I/O mapped memory. Android binder is a IPC machanism which will
>> not access I/O memory.
>>
>> Also VM_IOREMAP has alignment requiement which may not needed in
>> binder.
>> ----
>> __get_vm_area_node()
>> {
>> ...
>>     if (flags & VM_IOREMAP)
>>         align = 1ul << clamp_t(int, fls_long(size),
>>            PAGE_SHIFT, IOREMAP_MAX_ORDER);
>> ...
>> }
>> ----
>>
>> This patch use VM_ALLOC to get vm area.
>>
>> Signed-off-by: Ganesh Mahendran <opensource.ganesh@...il.com>
>> ---
>>  drivers/android/binder.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
>> index 16288e7..3511d5c 100644
>> --- a/drivers/android/binder.c
>> +++ b/drivers/android/binder.c
>> @@ -2885,7 +2885,7 @@ static int binder_mmap(struct file *filp, struct vm_area_struct *vma)
>>               goto err_already_mapped;
>>       }
>>
>> -     area = get_vm_area(vma->vm_end - vma->vm_start, VM_IOREMAP);
>> +     area = get_vm_area(vma->vm_end - vma->vm_start, VM_ALLOC);
>>       if (area == NULL) {
>>               ret = -ENOMEM;
>>               failure_string = "get_vm_area";
>
> What change have you noticed with this patch?  Have you tested it?
> Found that previously reserved iomemory is now free for binder to use
> where it wasn't?  What kind of change does the system now run as because
> of this?

I did some sanity test in Android system.  I will do more test using:
https://android.googlesource.com/platform/frameworks/native/+/master/libs/binder/tests/

>
> And are you _sure_ the alignment requirement isn't needed for binder?
> Have you verified this with the userspace binder library?

I will do the binder test.

Thanks.

>
> This is messy, tricky, stuff, I'm loath to change it without loads of
> testing having happened...
>
> thanks,
>
> greg k-h