On 8/26/19 11:36 AM, Laura Abbott wrote:
> On 8/23/19 10:28 PM, Alexey Skidanov wrote:
>> In ion_cma_heap, the allocated buffer is represented by a single
>> struct scatterlist instance. The length field of this struct is
>> 32 bit, hence the maximal size of requested buffer should be
>> less than 4GB.
>>
>> The len paramer of the allocation function is 64 bit (on 64 bit systems).
>> Hence the requested size might be greater than 4GB and in this case
>> the field length of the struct scatterlist is initialized incorrectly.
>>
>> To fix this, we check that requested size may fit into
>> the field length of the struct scatterlist
>>
>
> Is this a real issue that's actually possible to hit? Allocating
> more than a 4GB region of CMA seems ill advised and likely to
> throw off all the accounting.
>
Yes. Not sure why it seems ill advised - most of the buffers are small or 
middle size ones, but sometimes really huge one is requested.

>> Signed-off-by: Alexey Skidanov <alexey.skida...@intel.com>
>> ---
>>   drivers/staging/android/ion/ion.h          | 5 +++++
>>   drivers/staging/android/ion/ion_cma_heap.c | 3 +++
>>   2 files changed, 8 insertions(+)
>>
>> diff --git a/drivers/staging/android/ion/ion.h 
>> b/drivers/staging/android/ion/ion.h
>> index e291299..9dd7e20 100644
>> --- a/drivers/staging/android/ion/ion.h
>> +++ b/drivers/staging/android/ion/ion.h
>> @@ -21,6 +21,11 @@
>>     #include "../uapi/ion.h"
>>   +#define MAX_SCATTERLIST_LEN ({\
>> +        typeof(((struct scatterlist *)0)->length) v;\
>> +        v = -1;\
>> +    })
>> +
>>   /**
>>    * struct ion_buffer - metadata for a particular buffer
>>    * @node:        node in the ion_device buffers tree
>> diff --git a/drivers/staging/android/ion/ion_cma_heap.c 
>> b/drivers/staging/android/ion/ion_cma_heap.c
>> index bf65e67..d069719 100644
>> --- a/drivers/staging/android/ion/ion_cma_heap.c
>> +++ b/drivers/staging/android/ion/ion_cma_heap.c
>> @@ -36,6 +36,9 @@ static int ion_cma_allocate(struct ion_heap *heap, struct 
>> ion_buffer *buffer,
>>       unsigned long align = get_order(size);
>>       int ret;
>>   +    if (size > MAX_SCATTERLIST_LEN)
>> +        return -EINVAL;
>> +
>>       if (align > CONFIG_CMA_ALIGNMENT)
>>           align = CONFIG_CMA_ALIGNMENT;
>>  
>
_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Reply via email to