diff mbox series

[5/5] zfs: Fix zfs_read() to actually work

Message ID 20240407014743.13872-6-mwleeds@mailtundra.com
State Accepted
Delegated to: Tom Rini
Headers show
Series zfs: Fix zfs support on aarch64 | expand

Commit Message

mwleeds@mailtundra.com April 7, 2024, 1:47 a.m. UTC 
Without this patch, the while loop being modified goes on infinitely,
but with the patch I am able to boot linux on zfs on a jetson tx2 nx.

It seems like this code was never tested because the logic is clearly
wrong. The function do_div(a,b) does a division that modifies the first
parameter to have a = a / b, and returns the remainder of the division.
So clearly in the usual case when file->offset = 0, the line
"blkid = do_div(blkid, blksz);" just results in blkid being set to zero
on every iteration of the loop, rather than being incremented as blocks
are read. Hence the zeroth block is read over and over and this becomes
an infinite loop.

So instead capture the remainder of the division in a "blkoff" variable,
and use that to properly calculate the memory address to move from in
memmove() below.

For example, if file->offset were 1337, on the first iteration of the
loop blkid would be 0 and blkoff would be 1337. If the blksz is 131072
(as it was for me), that amount of data would be copied into
data->file_buf. movesize would be 131072 - 1337 = 129735 so 129735 bytes
would be moved into buf. On the second iteration of the loop (assuming
there is one), red would be 129735, blkid would be 1, blkoff would be 0,
and 131072 bytes would be copied into buf. And so on...

Signed-off-by: Phaedrus Leeds <mwleeds@mailtundra.com>
Tested-by: Phaedrus Leeds <mwleeds@mailtundra.com>
---
 fs/zfs/zfs.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
diff mbox series

Patch

diff --git a/fs/zfs/zfs.c b/fs/zfs/zfs.c
index 9a50deac18..bfc11fa667 100644
--- a/fs/zfs/zfs.c
+++ b/fs/zfs/zfs.c
@@ -2128,37 +2128,36 @@  zfs_read(zfs_file_t file, char *buf, uint64_t len)
 	 * now, this only reads in one data block at a time.
 	 */
 	length = len;
 	red = 0;
 	while (length) {
 		void *t;
 		/*
 		 * Find requested blkid and the offset within that block.
 		 */
 		uint64_t blkid = file->offset + red;
-		blkid = do_div(blkid, blksz);
+		uint64_t blkoff = do_div(blkid, blksz);
 		free(data->file_buf);
 		data->file_buf = 0;
 
 		err = dmu_read(&(data->dnode), blkid, &t,
 					   0, data);
 		data->file_buf = t;
 		if (err)
 			return -1;
 
 		data->file_start = blkid * blksz;
 		data->file_end = data->file_start + blksz;
 
 		movesize = min(length, data->file_end - (int)file->offset - red);
 
-		memmove(buf, data->file_buf + file->offset + red
-				- data->file_start, movesize);
+		memmove(buf, data->file_buf + blkoff, movesize);
 		buf += movesize;
 		length -= movesize;
 		red += movesize;
 	}
 
 	return len;
 }
 
 int
 zfs_close(zfs_file_t file)