| Message ID | 20190216072540.26272-1-zlang@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] fsstress: avoid infinite zero byte reading | expand |
On Sat, Feb 16, 2019 at 03:25:40PM +0800, Zorro Lang wrote: > copyrange_f and splice_f functions use a while loop to read a file, > it's fine if there's only one fsstress process(and its children), > but if some third part testing processes remove the file in the > middle phase of copyrange_f running, copyrange_f maybe always return > 0, and the while loop can't be end. As below: > > root 47184 xxxxxx S+ ./fsstress -R -d /mnt/scratch -n 10000 -p 20 -v > root 47187 xxxxxx R+ ./fsstress -d /mnt/scratch -n 10000 -p 20 -v > root 47199 xxxxxx R+ ./fsstress -d /mnt/scratch -n 10000 -p 20 -v > root 47314 xxxxxx S+ grep --color=auto fsstress > ... > ... > copy_file_range(3, [372258], 4, [2658770], 71179, 0) = 0 > copy_file_range(3, [372258], 4, [2658770], 71179, 0) = 0 > copy_file_range(3, [372258], 4, [2658770], 71179, 0) = 0 > copy_file_range(3, [372258], 4, [2658770], 71179, 0) = 0 > ... > ... > lr-x------. 1 root root 64 Jan 28 11:34 /proc/47187/fd/3 -> '/mnt/scratch/p2/f2 (deleted)' > > Signed-off-by: Zorro Lang <zlang@redhat.com> > --- > > V2 remove 300 times loop, just return if get ret == 0 > > Thanks, > Zorro > > ltp/fsstress.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/ltp/fsstress.c b/ltp/fsstress.c > index 25e0c3e2..138f7ecf 100644 > --- a/ltp/fsstress.c > +++ b/ltp/fsstress.c > @@ -2363,7 +2363,7 @@ copyrange_f( > int v2; > int fd1; > int fd2; > - size_t ret; > + size_t ret = 0; > int e; What's this fix? > > /* Load paths */ > @@ -2452,7 +2452,7 @@ copyrange_f( > if (ret < 0) { > if (errno != EAGAIN || tries++ >= 300) > break; > - } else if (ret > len) > + } else if (ret > len || ret == 0) Shoul dbe changing the first line, not this one. - if (ret < 0) { + if (ret <= 0) { > break; > else if (ret > 0) > len -= ret; > @@ -2908,6 +2908,9 @@ splice_f(int opno, long r) > > len -= ret1; > total += ret1; > + if (ret1 == 0) { > + break; > + } Same thing as above. The checks after the splice() calls should be changed from "if (ret < 0)" to "if (ret <= 0)". And there's two splice calls with different error return values that need to be checked, not one. And there's two checks for ret2, not one. :P Cheers, Dave.
diff --git a/ltp/fsstress.c b/ltp/fsstress.c index 25e0c3e2..138f7ecf 100644 --- a/ltp/fsstress.c +++ b/ltp/fsstress.c @@ -2363,7 +2363,7 @@ copyrange_f( int v2; int fd1; int fd2; - size_t ret; + size_t ret = 0; int e; /* Load paths */ @@ -2452,7 +2452,7 @@ copyrange_f( if (ret < 0) { if (errno != EAGAIN || tries++ >= 300) break; - } else if (ret > len) + } else if (ret > len || ret == 0) break; else if (ret > 0) len -= ret; @@ -2908,6 +2908,9 @@ splice_f(int opno, long r) len -= ret1; total += ret1; + if (ret1 == 0) { + break; + } } if (ret1 < 0 || ret2 < 0)
copyrange_f and splice_f functions use a while loop to read a file, it's fine if there's only one fsstress process(and its children), but if some third part testing processes remove the file in the middle phase of copyrange_f running, copyrange_f maybe always return 0, and the while loop can't be end. As below: root 47184 xxxxxx S+ ./fsstress -R -d /mnt/scratch -n 10000 -p 20 -v root 47187 xxxxxx R+ ./fsstress -d /mnt/scratch -n 10000 -p 20 -v root 47199 xxxxxx R+ ./fsstress -d /mnt/scratch -n 10000 -p 20 -v root 47314 xxxxxx S+ grep --color=auto fsstress ... ... copy_file_range(3, [372258], 4, [2658770], 71179, 0) = 0 copy_file_range(3, [372258], 4, [2658770], 71179, 0) = 0 copy_file_range(3, [372258], 4, [2658770], 71179, 0) = 0 copy_file_range(3, [372258], 4, [2658770], 71179, 0) = 0 ... ... lr-x------. 1 root root 64 Jan 28 11:34 /proc/47187/fd/3 -> '/mnt/scratch/p2/f2 (deleted)' Signed-off-by: Zorro Lang <zlang@redhat.com> --- V2 remove 300 times loop, just return if get ret == 0 Thanks, Zorro ltp/fsstress.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)