* [PATCH] xfsdump: fix bug restricting -f pathname length
@ 2011-02-24 18:40 Bill Kendall
2011-02-25 19:27 ` Alex Elder
0 siblings, 1 reply; 3+ messages in thread
From: Bill Kendall @ 2011-02-24 18:40 UTC (permalink / raw)
To: xfs
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.
Signed-off-by: Bill Kendall <wkendall@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;
}
--
1.7.0.4
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] xfsdump: fix bug restricting -f pathname length
2011-02-24 18:40 [PATCH] xfsdump: fix bug restricting -f pathname length Bill Kendall
@ 2011-02-25 19:27 ` Alex Elder
2011-02-25 19:55 ` Bill Kendall
0 siblings, 1 reply; 3+ messages in thread
From: Alex Elder @ 2011-02-25 19:27 UTC (permalink / raw)
To: Bill Kendall; +Cc: xfs
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?
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@sgi.com>
> Signed-off-by: Bill Kendall <wkendall@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;
> }
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] xfsdump: fix bug restricting -f pathname length
2011-02-25 19:27 ` Alex Elder
@ 2011-02-25 19:55 ` Bill Kendall
0 siblings, 0 replies; 3+ messages in thread
From: Bill Kendall @ 2011-02-25 19:55 UTC (permalink / raw)
To: aelder; +Cc: xfs
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@sgi.com>
>
>> Signed-off-by: Bill Kendall<wkendall@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;
>> }
>
>
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2011-02-25 19:52 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-02-24 18:40 [PATCH] xfsdump: fix bug restricting -f pathname length Bill Kendall
2011-02-25 19:27 ` Alex Elder
2011-02-25 19:55 ` Bill Kendall
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox