Re: [PATCH] fsx: add more check for copy_file_range

[Date Prev] [Date Next] [Thread Prev] [Thread Next] [Date Index] [Thread Index]




在 2019/9/25 21:08, Eryu Guan 写道:
On Wed, Sep 25, 2019 at 03:16:55PM +0800, Su Yanjun wrote:
On some linux distros(RHEL7, centos 7) copy_file_range uses
general implementation (splice interface). splice interace
uses pipe_to_file. pipe_to_file only work for different page.
The userspace cant's be  aware of such error because copy_file_range
returns ok too.
This looks like a kernel bug to me.
Yes, it's RHEL kernel 3.10's bug. The new kernel is ok.
So for such case when copy_file_range return we read back data
then check it.
Yeah, I think we should make sure the data copied is correct.

Signed-off-by: Su Yanjun <suyj.fnst@xxxxxxxxxxxxxx>
---
  ltp/fsx.c | 32 ++++++++++++++++++++++++++++++++
  1 file changed, 32 insertions(+)

diff --git a/ltp/fsx.c b/ltp/fsx.c
index 06d08e4..0439430 100644
--- a/ltp/fsx.c
+++ b/ltp/fsx.c
@@ -1602,6 +1602,7 @@ do_copy_range(unsigned offset, unsigned length, unsigned dest)
  	size_t olen;
  	ssize_t nr;
  	int tries = 0;
+	int ret = 0;
if (length == 0) {
  		if (!quiet && testcalls > simulatedopcount)
@@ -1665,6 +1666,37 @@ do_copy_range(unsigned offset, unsigned length, unsigned dest)
  		memset(good_buf + file_size, '\0', dest - file_size);
  	if (dest + length > file_size)
  		file_size = dest + length;
+	/*
+	 * Although copy_file_range returns ok
+	 * for some linux distros that use general implementation
+	 * (splice interface) such as RHEL 7, centos 7 that use
+	 * pipe_to_file will cause test fail.
+	 *
+	 * Here we add a little more check here for copy_file_range
+	 * We read copied data then check it. If check fail here
+	 * then report it.
+	 */
Above comments seem like not necessary, the content check is inspired by
Rthis HEL7 issue, but not the main purpose.

+	ret = lseek(fd, (off_t)dest, SEEK_SET);
+	if (ret == (off_t)-1) {
+		prterr("doread: lseek");
                         ^^^^^^^ not doread

+		report_failure(140);
Use a different failure number than that in doread, i.e. don't copy the
code from doread.

+	}
+	ret = fsxread(fd, temp_buf, length, dest);
+	if (ret != length) {
+		if (ret == -1)
+				prterr("doread: read");
+		else
+				prt("short read: 0x%x bytes instead of 0x%x\n",
+					ret, length);
Same here, not doread, and weired indention above.

+		report_failure(141);
+	}
+	if (memcmp(good_buf+dest, temp_buf, length) != 0) {
+		prt("copy range: 0x%x to 0x%x at 0x%x\n", offset,
+				offset + length, dest);
+		prterr("do_copy_range:");
+		report_failure(161);
I think we could take use of check_buffers().

And similar check could be added to do_clone_range as well, I think.

Ok, i'll send patch v2 later.

Thanks


Thanks,
Eryu

+
+	}
  }
#else
--
2.7.4









[Index of Archives]     [Linux Filesystems Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux