ACK/cmnt: [Disco][SRU][CVE-2019-2214] Fix for Binder OOB write

[Kamal Mostafa]

Backport LGTM, so...

Acked-by: Kamal Mostafa <kamal at canonical.com>

IMHO, your backport note in the commit does not need to contain such a
detailed picture of the backport (nor the whole original commit!).
A reviewer is likely going to go examine the actual upstream commit
anyway.  :-)

 -Kamal

On Tue, Nov 26, 2019 at 02:42:30PM -0800, Connor Kuehl wrote:
> https://people.canonical.com/~ubuntu-security/cve/2019/CVE-2019-2214.html
> 
> From the link above:
> 
>     "In binder_transaction of binder.c, there is a possible out of bounds write
>     due to a missing bounds check. This could lead to local escalation of
>     privilege with no additional execution privileges needed. User interaction
>     is not needed for exploitation.Product: AndroidVersions: Android
>     kernelAndroid ID: A-136210786References: Upstream kernel"
> 
> These backport notes are also included on the patch:
> 
> There is a much larger cleanup patch (that Disco does not have) that moves
> away from using pointers and pointer arithmetic to using uintptr values,
> that patch is bde4a19fc04f ("binder: use userspace pointer as base of
> buffer space"). I used that patch to create a mapping of equivalent
> variables:
> 
>   sg_bufp    => sg_buf_offset
>   sg_buf_end => sg_buf_end_offset
>   offp       => buffer_offset
>   off_start  => off_start_offset
>   off_end    => off_end_offset
> 
> to construct an equivalent patch for this CVE without pulling in the
> other larger patch to base this on.
> 
> No new compiler warnings for the android driver.
> 
> @@ -3239,7 +3239,8 @@ static void binder_transaction(struct binder_proc *proc,      
>     buffer_offset = off_start_offset;                                               
>     off_end_offset = off_start_offset + tr->offsets_size;                           
>     sg_buf_offset = ALIGN(off_end_offset, sizeof(void *));                          
> -   sg_buf_end_offset = sg_buf_offset + extra_buffers_size;                         
> +   sg_buf_end_offset = sg_buf_offset + extra_buffers_size -                        
> +       ALIGN(secctx_sz, sizeof(u64));                                              
>     off_min = 0;                                                                    
>     for (buffer_offset = off_start_offset; buffer_offset < off_end_offset;          
>          buffer_offset += sizeof(binder_size_t)) {
> 
> -- 
> kernel-team mailing list
> kernel-team at lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team