* [PATCH] fsr: fix uninitialized fs usage after timeout @ 2017-06-02 18:20 Jeff Mahoney 2017-06-21 19:28 ` Eric Sandeen 0 siblings, 1 reply; 3+ messages in thread From: Jeff Mahoney @ 2017-06-02 18:20 UTC (permalink / raw) To: linux-xfs 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. 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)); } -- Jeff Mahoney SUSE Labs ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] fsr: fix uninitialized fs usage after timeout 2017-06-02 18:20 [PATCH] fsr: fix uninitialized fs usage after timeout Jeff Mahoney @ 2017-06-21 19:28 ` Eric Sandeen 2017-06-21 21:49 ` Jeff Mahoney 0 siblings, 1 reply; 3+ messages in thread From: Eric Sandeen @ 2017-06-21 19:28 UTC (permalink / raw) To: Jeff Mahoney, linux-xfs 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. 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)); } ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] fsr: fix uninitialized fs usage after timeout 2017-06-21 19:28 ` Eric Sandeen @ 2017-06-21 21:49 ` Jeff Mahoney 0 siblings, 0 replies; 3+ messages in thread From: Jeff Mahoney @ 2017-06-21 21:49 UTC (permalink / raw) To: Eric Sandeen, linux-xfs [-- 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 --] ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-06-21 21:50 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-06-02 18:20 [PATCH] fsr: fix uninitialized fs usage after timeout Jeff Mahoney 2017-06-21 19:28 ` Eric Sandeen 2017-06-21 21:49 ` Jeff Mahoney
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).