RE: [PATCH] staging: android: ion: Fixed uninitialized heap name access
> -Original Message- > From: Dan Carpenter [mailto:dan.carpen...@oracle.com] > Sent: Tuesday, October 23, 2018 08:33 > To: Skidanov, Alexey > Cc: Laura Abbott ; gre...@linuxfoundation.org; > de...@driverdev.osuosl.org > Subject: Re: [PATCH] staging: android: ion: Fixed uninitialized heap name > access > > On Mon, Oct 22, 2018 at 05:47:08PM +0300, Alexey Skidanov wrote: > > > > > > On 10/22/18 17:32, Laura Abbott wrote: > > > On 10/22/2018 07:02 AM, Alexey Skidanov wrote: > > >> The heap name might be uninitialized and access might crash the > > >> kernel. > > >> > > > > > > The heap name should never be null so this seems like this is being > > > fixed in the wrong place. Can you explain more how you are hitting > > > this issue? > > Sure. Carve out heap name is uninitialized. There is the next patch > > fixing it. But to be on the safe side, I have added the check. > > > > You keep saying uninitialized but you mean NULL. I meant the uninitialized name, not the pointer. > > ion_carveout_heap_create() is never called so far as I can see so this > isn't an issue in real life. It feels like it would be detected right ion_carveout_heap_create() is the only way to create this kind of heap. You are correct that currently it's never called - it's designed to be called by board specific code and in the meanwhile there is no standard way to do it. > away when that code was used. We should just apply your follow on > patch instead. > > regards, > dan carpenter Thanks, Alexey ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: ion: Fixed uninitialized heap name access
On Mon, Oct 22, 2018 at 05:47:08PM +0300, Alexey Skidanov wrote: > > > On 10/22/18 17:32, Laura Abbott wrote: > > On 10/22/2018 07:02 AM, Alexey Skidanov wrote: > >> The heap name might be uninitialized and access might crash the > >> kernel. > >> > > > > The heap name should never be null so this seems like this is being > > fixed in the wrong place. Can you explain more how you are hitting > > this issue? > Sure. Carve out heap name is uninitialized. There is the next patch > fixing it. But to be on the safe side, I have added the check. > You keep saying uninitialized but you mean NULL. ion_carveout_heap_create() is never called so far as I can see so this isn't an issue in real life. It feels like it would be detected right away when that code was used. We should just apply your follow on patch instead. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: ion: Fixed uninitialized heap name access
On 10/22/18 17:32, Laura Abbott wrote:
> On 10/22/2018 07:02 AM, Alexey Skidanov wrote:
>> The heap name might be uninitialized and access might crash the
>> kernel.
>>
>
> The heap name should never be null so this seems like this is being
> fixed in the wrong place. Can you explain more how you are hitting
> this issue?
Sure. Carve out heap name is uninitialized. There is the next patch
fixing it. But to be on the safe side, I have added the check.
Thanks,
Alexey
>
> Thanks,
> Laura
>
>> Signed-off-by: Alexey Skidanov
>> ---
>> drivers/staging/android/ion/ion.c | 7 +--
>> 1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/staging/android/ion/ion.c
>> b/drivers/staging/android/ion/ion.c
>> index 9907332..55bca92d 100644
>> --- a/drivers/staging/android/ion/ion.c
>> +++ b/drivers/staging/android/ion/ion.c
>> @@ -459,8 +459,11 @@ int ion_query_heaps(struct ion_heap_query *query)
>> max_cnt = query->cnt;
>> plist_for_each_entry(heap, >heaps, node) {
>> - strncpy(hdata.name, heap->name, MAX_HEAP_NAME);
>> - hdata.name[sizeof(hdata.name) - 1] = '\0';
>> + if (heap->name) {
>> + strncpy(hdata.name, heap->name, MAX_HEAP_NAME);
>> + hdata.name[sizeof(hdata.name) - 1] = '\0';
>> + }
>> +
>> hdata.type = heap->type;
>> hdata.heap_id = heap->id;
>>
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: ion: Fixed uninitialized heap name access
On 10/22/2018 07:02 AM, Alexey Skidanov wrote:
The heap name might be uninitialized and access might crash the
kernel.
The heap name should never be null so this seems like this is being
fixed in the wrong place. Can you explain more how you are hitting
this issue?
Thanks,
Laura
Signed-off-by: Alexey Skidanov
---
drivers/staging/android/ion/ion.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index 9907332..55bca92d 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -459,8 +459,11 @@ int ion_query_heaps(struct ion_heap_query *query)
max_cnt = query->cnt;
plist_for_each_entry(heap, >heaps, node) {
- strncpy(hdata.name, heap->name, MAX_HEAP_NAME);
- hdata.name[sizeof(hdata.name) - 1] = '\0';
+ if (heap->name) {
+ strncpy(hdata.name, heap->name, MAX_HEAP_NAME);
+ hdata.name[sizeof(hdata.name) - 1] = '\0';
+ }
+
hdata.type = heap->type;
hdata.heap_id = heap->id;
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: android: ion: Fixed uninitialized heap name access
The heap name might be uninitialized and access might crash the
kernel.
Signed-off-by: Alexey Skidanov
---
drivers/staging/android/ion/ion.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index 9907332..55bca92d 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -459,8 +459,11 @@ int ion_query_heaps(struct ion_heap_query *query)
max_cnt = query->cnt;
plist_for_each_entry(heap, >heaps, node) {
- strncpy(hdata.name, heap->name, MAX_HEAP_NAME);
- hdata.name[sizeof(hdata.name) - 1] = '\0';
+ if (heap->name) {
+ strncpy(hdata.name, heap->name, MAX_HEAP_NAME);
+ hdata.name[sizeof(hdata.name) - 1] = '\0';
+ }
+
hdata.type = heap->type;
hdata.heap_id = heap->id;
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
