Re: [PATCH 1/1] xfs: Fix Smatch warning in xfs_attr_node_get

[Allison Collins <allison.henderson@oracle.com>]

On 7/28/20 6:23 PM, Darrick J. Wong wrote:
> On Tue, Jul 28, 2020 at 05:08:53PM -0700, Allison Collins wrote:
>> Fix warning: variable dereferenced before check 'state' in
>> xfs_attr_node_get.  If xfs_attr_node_hasname fails, it may return a null
>> state.  If state is null, do not derefrence it.  Go straight to out.
>>
>> Signed-off-by: Allison Collins <allison.henderson@oracle.com>
>> ---
>>   fs/xfs/libxfs/xfs_attr.c | 5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
>> index e5ec9ed..90b7b24 100644
>> --- a/fs/xfs/libxfs/xfs_attr.c
>> +++ b/fs/xfs/libxfs/xfs_attr.c
>> @@ -1409,6 +1409,9 @@ xfs_attr_node_get(
>>   	 * Search to see if name exists, and get back a pointer to it.
>>   	 */
>>   	error = xfs_attr_node_hasname(args, &state);
>> +	if (!state)
>> +		goto out;
>> +
>>   	if (error != -EEXIST)
>>   		goto out_release;
>>   
>> @@ -1426,7 +1429,7 @@ xfs_attr_node_get(
> 
> I would've just changed the for loop to:
> 
> 	for (i = 0; state && i < state->path.active; i++) {
> 
> Since that way we'd know that the error-out path always does the right
> thing wrt any resources that could have been allocated.
> 
Sure, will update

Allison
> --D
> 
>>   		xfs_trans_brelse(args->trans, state->path.blk[i].bp);
>>   		state->path.blk[i].bp = NULL;
>>   	}
>> -
>> +out:
>>   	if (state)
>>   		xfs_da_state_free(state);
>>   	return error;
>> -- 
>> 2.7.4
>>