Re: [PATCH] fsr: fix uninitialized fs usage after timeout

[Jeff Mahoney <jeffm@suse.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2444 bytes --]

On 6/21/17 3:28 PM, Eric Sandeen wrote:
> On 6/2/17 1:20 PM, Jeff Mahoney wrote:
>> In the main loop of fsrallfs, we exit when we've hit the timeout but
>> we increment fs before we get there.  If we're operating on the last
>> file system in the array, we'll hit an uninitialized fsdesc and
>> crash in fsrall_cleanup.
> 
> Ugh, really - nobody should be using the defrag-the-world mode,
> but we ship it, so ...
> 
>> Signed-off-by: Jeff Mahoney <jeffm@suse.com>
>> ---
>>  fsr/xfs_fsr.c | 4 +---
>>  1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/fsr/xfs_fsr.c b/fsr/xfs_fsr.c
>> index 517b75f0..e695c243 100644
>> --- a/fsr/xfs_fsr.c
>> +++ b/fsr/xfs_fsr.c
>> @@ -598,7 +598,7 @@ fsrallfs(char *mtab, int howlong, char *leftofffile)
>>  	signal(SIGTERM, aborter);
>>
>>  	/* reorg for 'howlong' -- checked in 'fsrfs' */
>> -	while (endtime > time(0)) {
>> +	for (; endtime > time(0); fs->npass++, fs++) {
>>  		pid_t pid;
>>  		if (fs == fsend)
>>  			fs = fsbase;
>> @@ -629,8 +629,6 @@ fsrallfs(char *mtab, int howlong, char *leftofffile)
>>  			break;
>>  		}
>>  		startino = 0;  /* reset after the first time through */
>> -		fs->npass++;
>> -		fs++;
>>  	}
>>  	fsrall_cleanup(endtime <= time(0));
>>  }
> 
> I hate to be that PITA maintainer who only wants to do it his way ;) but
> would this be any tidier?
> 
> I'm just not that big a fan of "for(; ....)" loops.

Sure, this'll work just as well.

-Jeff

> diff --git a/fsr/xfs_fsr.c b/fsr/xfs_fsr.c
> index 517b75f..3a5f683 100644
> --- a/fsr/xfs_fsr.c
> +++ b/fsr/xfs_fsr.c
> @@ -600,12 +600,6 @@ fsrallfs(char *mtab, int howlong, char *leftofffile)
>  	/* reorg for 'howlong' -- checked in 'fsrfs' */
>  	while (endtime > time(0)) {
>  		pid_t pid;
> -		if (fs == fsend)
> -			fs = fsbase;
> -		if (fs->npass == npasses) {
> -			fsrprintf(_("Completed all %d passes\n"), npasses);
> -			break;
> -		}
>  		if (npasses > 1 && !fs->npass)
>  			Mflag = 1;
>  		else
> @@ -631,6 +625,12 @@ fsrallfs(char *mtab, int howlong, char *leftofffile)
>  		startino = 0;  /* reset after the first time through */
>  		fs->npass++;
>  		fs++;
> +		if (fs == fsend)
> +			fs = fsbase;
> +		if (fs->npass == npasses) {
> +			fsrprintf(_("Completed all %d passes\n"), npasses);
> +			break;
> +		}
>  	}
>  	fsrall_cleanup(endtime <= time(0));
>  }
> 
> 
> 


-- 
Jeff Mahoney
SUSE Labs


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]