Re: Re: [RFC PATCH] staging/android/ion : fix a race condition in the ion driver

Re: Re: [RFC PATCH] staging/android/ion : fix a race condition in the ion driver

[EunTaik Lee]

2016-02-18 3:54 GMT+09:00 Laura Abbott <labbott@redhat.com>:
> On 02/16/2016 10:32 PM, EunTaik Lee wrote:
>> There was a use-after-free problem in the ion driver.
>>
>> The problem is detected as an unaligned access in the
>> spin lock functions since it uses load exclusive
>>   instruction. In some cases it corrupts the slub's
>> free pointer which causes a unaligned access to the
>> next free pointer.(thus the kmalloc function returns
>> a pointer like ffffc0745b4580aa). And it causes lots of other
>> hard-to-debug problems.
>>
>> This symptom is caused since the first member in the
>> ion_handle structure is the reference count and the
>> ion driver decrements the reference after it has been
>> freed.
>>
>> To fix this problem client->lock mutex is extended
>> to protect all the codes that uses the handle.
>>
>
> This describes the symptoms very well but what's the actual
> race condition?

The actual race condition was in case ION_IOC_FREE of ion_ioctl()
function.

A handle has ref count of 1 and two tasks on different
cpus calls ION_IOC_FREE simultaneously.

cpu 0					cpu 1
-------------------------------------------------------
ion_handle_get_by_id()
(ref == 2)
				ion_handle_get_by_id()
				(ref == 3)

ion_free()
(ref == 2)

ion_handle_put()
(ref == 1)

				ion_free()
				(ref == 0 so ion_handle_destroy() is called
				 and the handle is freed.)
				
				ion_handle_put() is called and it
				decreases the slub's next free pointer

So it's basically a double free on the userspace.
But I made this patch since I don't think we should 
take a risk of such a malicious code to mess up
the kernel.

> Also what tree did you generate this against? It doesn't
> seem to apply.

The patch was generated against the stable-kernel.
I will resend the patch using the latest stable-kernel
along with a silly mistake that I've made when merging the
patch(made against 3.18.20) to the stable-kernel.
(Thank you Julia <julia.lawall@lip6.fr> for pointing out
 my mistake)