[PATCH] xfsdump: fix bug restricting -f pathname length

Bill Kendall wkendall at sgi.com
Fri Feb 25 13:55:22 CST 2011


On 02/25/2011 01:27 PM, Alex Elder wrote:
> On Thu, 2011-02-24 at 12:40 -0600, Bill Kendall wrote:
>> A temporary buffer storing the dump filename (-f) is smaller than
>> the otherwise allowed pathname length. This buffer is used to
>> extract the pathname from additional parameters specified as part
>> of the -f argument. As these parameters are an undocumented and
>> unused extension of the -f argument, simply remove this code.
>> Note that drive_alloc() always dups the given pathname, so
>> 'optarg' is passed directly.
>
> If someone happens to put something starting with a comma
> after the device path name, will it simply fail because
> a device by that name (comma included) won't exist?

xfsdump creates the file if it does not exist, so you would
end up with a comma in the dump filename.

Thanks,
Bill

>
> Either way, this looks good.  I'll hold off a bit before
> taking this in, in case someone else wants a chance to
> review.
>
> Reviewed-by: Alex Elder<aelder at sgi.com>
>
>> Signed-off-by: Bill Kendall<wkendall at sgi.com>
>> ---
>>   common/drive.c |   18 +-----------------
>>   1 files changed, 1 insertions(+), 17 deletions(-)
>>
>> diff --git a/common/drive.c b/common/drive.c
>> index f184afa..227fecd 100644
>> --- a/common/drive.c
>> +++ b/common/drive.c
>> @@ -137,10 +137,6 @@ drive_init1( int argc, char *argv[ ], bool_t singlethreaded )
>>   	opterr = 0;
>>   	driveix = 0;
>>   	while ( ( c = getopt( argc, argv, GETOPT_CMDSTRING )) != EOF ) {
>> -		char optarray[100];
>> -		char *devname;
>> -		char *token;
>> -
>>   		switch ( c ) {
>>   		case GETOPT_DUMPDEST:
>>   			if ( ! optarg || optarg[ 0 ] == '-' ) {
>> @@ -151,21 +147,9 @@ drive_init1( int argc, char *argv[ ], bool_t singlethreaded )
>>   				return BOOL_FALSE;
>>   			}
>>
>> -			/* remove the device name from the rest of the
>> -			 * parameter string. note that strdup malloc()s
>> -			 * a string; important since optarray is an auto.
>> -			 */
>> -			ASSERT( strlen( optarg )<  sizeof( optarray ));
>> -			strncpy( optarray, optarg, sizeof( optarray ));
>> -			optarray[ sizeof( optarray ) - 1 ] = 0;
>> -			if ( ( token = strtok( optarray, "," )) == NULL ) {
>> -				token = optarray;
>> -			}
>> -			devname = strdup( token );
>> -
>>   			/* allocate a drive descriptor
>>   			 */
>> -			drivepp[ driveix ] = drive_alloc( devname, driveix );
>> +			drivepp[ driveix ] = drive_alloc( optarg, driveix );
>>   			driveix++;
>>   			break;
>>   		}
>
>




More information about the xfs mailing list