Re: [PATCH] fs:Fix kmemleak leak warning in getname_flags about working on unitialized memory

[nick <xerofoify@gmail.com>]

On 2016-08-04 09:31 AM, Vlastimil Babka wrote:
> On 08/03/2016 11:48 PM, Nicholas Krause wrote:
>> This fixes a kmemleak leak warning complaining about working on
>> unitializied memory as found in the function, getname_flages. Seems
> 
> What exactly is the kmemleak warning saying?
> 
>> that we are indeed working on unitialized memory, as the filename
>> char pointer is never made to point to the filname structure's result
>> member for holding it's name, fix this by using memcpy to copy the
>> filname structure pointer's, name to the char pointer passed to this
>> function.
> 
> I don't understand what you're saying here. "the char pointer passed to
> this function" is the source, not destination.
> 
That's fine what I mean to state is this we are never copying back our internal
struct filename result's name member to the user pointer leading to a kmemleak
warning.
>> Signed-off-by: Nicholas Krause <xerofoify@gmail.com>
>> ---
>>  fs/namei.c         | 1 +
>>  mm/early_ioremap.c | 1 +
>>  2 files changed, 2 insertions(+)
>>
>> diff --git a/fs/namei.c b/fs/namei.c
>> index c386a32..6b18d57 100644
>> --- a/fs/namei.c
>> +++ b/fs/namei.c
>> @@ -196,6 +196,7 @@ getname_flags(const char __user *filename, int flags, int *empty)
>>  		}
>>  	}
>>  
>> +	memcpy((char *)result->name, filename, len);
> 
> This will be wrong even with strncpy_from_user instead of memcpy. AFAICS
> result->name already points to a copy of filename.
Yes that is correct but the pointer we are passing is called, filename into
getname_flags which is what I am passing as the second argument which is
confusing at least to me :).
> Also if you think that the above is "copy[ing] the filname structure
> pointer's, name to the char pointer passed to this function" then you
> are wrong.
> 
I assumed here that it was copying or moving the pointer over to point to
the region of memory allocated for the structure result pointer to hold
it's name member, I could be wrong :).
>>  	result->uptr = filename;
>>  	result->aname = NULL;
>>  	audit_getname(result);
>> diff --git a/mm/early_ioremap.c b/mm/early_ioremap.c
>> index 6d5717b..92c5235 100644
>> --- a/mm/early_ioremap.c
>> +++ b/mm/early_ioremap.c
>> @@ -215,6 +215,7 @@ early_ioremap(resource_size_t phys_addr, unsigned long size)
>>  void __init *
>>  early_memremap(resource_size_t phys_addr, unsigned long size)
>>  {
>> +	dump_stack();
>>  	return (__force void *)__early_ioremap(phys_addr, size,
>>  					       FIXMAP_PAGE_NORMAL);
>>  }
>>
> 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>