linux-kernel - Re: [PATCH 11/14] staging: android: ion: Allow heap name to be null

Open Source and information security mailing list archives

Message-ID: <39453e0a-2db8-3a59-1909-e02caa9afa31@ti.com>
Date:   Wed, 16 Jan 2019 11:12:28 -0600
From:   "Andrew F. Davis" <afd@...com>
To:     Brian Starkey <Brian.Starkey@....com>
CC:     Laura Abbott <labbott@...hat.com>,
        Sumit Semwal <sumit.semwal@...aro.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Arve Hjønnevåg <arve@...roid.com>,
        "devel@...verdev.osuosl.org" <devel@...verdev.osuosl.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
        nd <nd@....com>
Subject: Re: [PATCH 11/14] staging: android: ion: Allow heap name to be null

On 1/16/19 9:28 AM, Brian Starkey wrote:
> Hi Andrew,
> 
> On Fri, Jan 11, 2019 at 12:05:20PM -0600, Andrew F. Davis wrote:
>> The heap name can be used for debugging but otherwise does not seem
>> to be required and no other part of the code will fail if left NULL
>> except here. We can make it required and check for it at some point,
>> for now lets just prevent this from causing a NULL pointer exception.
> 
> I'm not so keen on this one. In the "new" API with heap querying, the
> name string is the only way to identify the heap. I think Laura
> mentioned at XDC2017 that it was expected that userspace should use
> the strings to find the heap they want.
> 

Right now the names are only for debug. I accidentally left the name
null once and got a kernel crash. This is the only spot where it is
needed so I fixed it up. The other option is to make the name mandatory
and properly error out, I don't want to do that right now until the
below discussion is had to see if names really do matter or not.

> I'd actually be in favor of making the string a more strict UAPI than
> allowing it to be empty (at least, if heap name strings is the API we
> decide on for identifying heaps - which is another discussion).
> 

I would think identifying heaps by name would be less portable, but as
you said that is a whole different discussion..

Thanks,
Andrew

> Cheers,
> -Brian
> 
>>
>> Signed-off-by: Andrew F. Davis <afd@...com>
>> ---
>>  drivers/staging/android/ion/ion.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
>> index bba5f682bc25..14e48f6eb734 100644
>> --- a/drivers/staging/android/ion/ion.c
>> +++ b/drivers/staging/android/ion/ion.c
>> @@ -467,7 +467,7 @@ static int ion_query_heaps(struct ion_heap_query *query)
>>  	max_cnt = query->cnt;
>>  
>>  	plist_for_each_entry(heap, &dev->heaps, node) {
>> -		strncpy(hdata.name, heap->name, MAX_HEAP_NAME);
>> +		strncpy(hdata.name, heap->name ?: "(null)", MAX_HEAP_NAME);
>>  		hdata.name[sizeof(hdata.name) - 1] = '\0';
>>  		hdata.type = heap->type;
>>  		hdata.heap_id = heap->id;
>> -- 
>> 2.19.1
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@...ts.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel

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