Re: [PATCH v2] erofs-utils: fuse: fix random readlink error

[Gao Xiang via Linux-erofs <linux-erofs@lists.ozlabs.org>]

Hi Weiwen,

On Sat, Jan 30, 2021 at 02:07:47AM +0800, Hu Weiwen wrote:
> readlink should fill a **null-terminated** string in the buffer.
> 
> To achieve this:
> 1) memset(0) for unmapped extents;
> 2) make erofsfuse_read() properly returning the actual bytes read;
> 3) insert a null character if the path is truncated.
> 
> Link: https://lore.kernel.org/r/20210121101233.GC6680@DESKTOP-N4CECTO.huww98.cn

Looked into this patch just now...

The Link tag is only used for refering to the patch itself.

> Signed-off-by: Hu Weiwen <sehuww@mail.scut.edu.cn>
> ---

...

>  
> @@ -91,9 +92,13 @@ static int erofs_read_raw_data(struct erofs_inode *inode, char *buffer,
>  
>  		if (!(map.m_flags & EROFS_MAP_MAPPED)) {
>  			if (!map.m_llen) {
> +				/* reached EOF */
> +				memset(buffer + ptr - offset, 0,
> +				       offset + size - ptr);
>  				ptr = offset + size;
>  				continue;
>  			}
> +			memset(buffer + map.m_la - offset, 0, map.m_llen);

There might be some minor issue --- `offset' could be larger than
`map.m_la' if sparse file is supported then.

I made an update version of this to fix these (some cleanup is
included as well), it would be nice of you to take another look at
this as well...

Thanks,
Gao Xiang

From bfbd8ee056aca57a77034b8723f3f828f806747b Mon Sep 17 00:00:00 2001
From: Hu Weiwen <sehuww@mail.scut.edu.cn>
Date: Sat, 30 Jan 2021 02:07:47 +0800
Subject: [PATCH v3] erofs-utils: fuse: fix random readlink error

readlink should fill a **null-terminated** string in the buffer [1].

To achieve this:
1) memset(0) for unmapped extents;
2) make erofsfuse_read() properly returning the actual bytes read;
3) insert a null character if the path is truncated.

[1] https://lore.kernel.org/r/20210121101233.GC6680@DESKTOP-N4CECTO.huww98.cn
Signed-off-by: Hu Weiwen <sehuww@mail.scut.edu.cn>
Signed-off-by: Gao Xiang <hsiangkao@aol.com>
---
 fuse/main.c |  8 ++++++++
 lib/data.c  | 20 ++++++++++++--------
 2 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/fuse/main.c b/fuse/main.c
index c16291272e75..37119ea8728d 100644
--- a/fuse/main.c
+++ b/fuse/main.c
@@ -74,6 +74,10 @@ static int erofsfuse_read(const char *path, char *buffer,
 	ret = erofs_pread(&vi, buffer, size, offset);
 	if (ret)
 		return ret;
+	if (offset + size > vi.i_size)
+		return vi.i_size - offset;
+	if (offset >= vi.i_size)
+		return 0;
 	return size;
 }
 
@@ -83,6 +87,10 @@ static int erofsfuse_readlink(const char *path, char *buffer, size_t size)
 
 	if (ret < 0)
 		return ret;
+	DBG_BUGON(ret > size);
+	if (ret == size)
+		buffer[size - 1] = '\0';
+	erofs_dbg("readlink(%s): %s", path, buffer);
 	return 0;
 }
 
diff --git a/lib/data.c b/lib/data.c
index 3781846743aa..5bc525f19826 100644
--- a/lib/data.c
+++ b/lib/data.c
@@ -29,6 +29,7 @@ static int erofs_map_blocks_flatmode(struct erofs_inode *inode,
 	if (offset >= inode->i_size) {
 		/* leave out-of-bound access unmapped */
 		map->m_flags = 0;
+		map->m_plen = 0;
 		goto out;
 	}
 
@@ -80,6 +81,7 @@ static int erofs_read_raw_data(struct erofs_inode *inode, char *buffer,
 	erofs_off_t ptr = offset;
 
 	while (ptr < offset + size) {
+		char *const estart = buffer + ptr - offset;
 		erofs_off_t eend;
 
 		map.m_la = ptr;
@@ -89,29 +91,30 @@ static int erofs_read_raw_data(struct erofs_inode *inode, char *buffer,
 
 		DBG_BUGON(map.m_plen != map.m_llen);
 
+		/* trim extent */
+		eend = min(offset + size, map.m_la + map.m_llen);
+		DBG_BUGON(ptr < map.m_la);
+
 		if (!(map.m_flags & EROFS_MAP_MAPPED)) {
 			if (!map.m_llen) {
+				/* reached EOF */
+				memset(estart, 0, offset + size - ptr);
 				ptr = offset + size;
 				continue;
 			}
-			ptr = map.m_la + map.m_llen;
+			memset(estart, 0, eend - ptr);
+			ptr = eend;
 			continue;
 		}
 
-		/* trim extent */
-		eend = min(offset + size, map.m_la + map.m_llen);
-		DBG_BUGON(ptr < map.m_la);
-
 		if (ptr > map.m_la) {
 			map.m_pa += ptr - map.m_la;
 			map.m_la = ptr;
 		}
 
-		ret = dev_read(buffer + ptr - offset,
-			       map.m_pa, eend - map.m_la);
+		ret = dev_read(estart, map.m_pa, eend - map.m_la);
 		if (ret < 0)
 			return -EIO;
-
 		ptr = eend;
 	}
 	return 0;
@@ -138,6 +141,7 @@ static int z_erofs_read_data(struct erofs_inode *inode, char *buffer,
 			return ret;
 
 		if (!(map.m_flags & EROFS_MAP_MAPPED)) {
+			memset(buffer + map.m_la - offset, 0, map.m_llen);
 			end = map.m_la;
 			continue;
 		}
-- 
2.24.0