[fedfs-utils] [PATCH 2/8] fedfsd: don't return freed memory through **pathname parameter, ...

[Jim Meyering]

Chuck Lever wrote:

> On Nov 30, 2011, at 3:51 PM, Jim Meyering wrote:
>
>> From: Jim Meyering <meyering at redhat.com>
>>
>> even though upon error it will not be used.
>> * src/fedfsd/svc.c (fedfsd_pathwalk): On an error path, don't
>> set *pathname at all, and certainly not to a just-freed pointer.
>>
>> mount avoid one-byte heap-write overrun
>> ---
>> src/fedfsd/svc.c |    1 -
>> 1 files changed, 0 insertions(+), 1 deletions(-)
>>
>> diff --git a/src/fedfsd/svc.c b/src/fedfsd/svc.c
>> index 132be70..3509082 100644
>> --- a/src/fedfsd/svc.c
>> +++ b/src/fedfsd/svc.c
>> @@ -318,7 +318,6 @@ fedfsd_pathwalk(const FedFsPathName fpath, char **pathname)
>> 		retval = fedfsd_pathwalk_check_term(result);
>> 		if (retval != FEDFS_OK)
>> 			free(result);
>> -		*pathname = result;
>
> if retval is FEDFS_OK, we want to preserve the result.
> So the existing code is incorrect, but this fix is also wrong.  How about

Oh.  How embarrassing.

> 	if (retval != FEDFS_OK) {
> 		free(result);
> 		return retval;
> 	}
> 	*pathname = result;
> 	return FEDFS_OK;
>
>> 		return retval;
>> 	}

How about this instead?
This way there's a single return point and no duplicated FEDFS_OK:

	if (fpath.FedFsPathName_len == 0) {
		xlog(D_CALL, "%s: Zero-component pathname", __func__);
		strcat(result, "/");
		retval = fedfsd_pathwalk_check_term(result);
		if (retval == FEDFS_OK)
			*pathname = result;
		else
			free(result);
		return retval;
	}