[v1] exfat: fix reporting fs error when reading dir beyond EOF

Message ID PUZPR04MB6316579893496BC54C4FE96F81EC9@PUZPR04MB6316.apcprd04.prod.outlook.com
State New
Headers
Series [v1] exfat: fix reporting fs error when reading dir beyond EOF |

Commit Message

Yuezhang.Mo@sony.com Dec. 26, 2022, 7:23 a.m. UTC
  Since seekdir() does not check whether the position is valid, the
position may exceed the size of the directory. We found that for
a directory with discontinuous clusters, if the position exceeds
the size of the directory and the excess size is greater than or
equal to the cluster size, exfat_readdir() will return -EIO,
causing a file system error and making the file system unavailable.

Reproduce this bug by:

seekdir(dir, dir_size + cluster_size);
dirent = readdir(dir);

The following log will be printed if mount with 'errors=remount-ro'.

[11166.712896] exFAT-fs (sdb1): error, invalid access to FAT (entry 0xffffffff)
[11166.712905] exFAT-fs (sdb1): Filesystem has been set read-only

Fixes: 1e5654de0f51 ("exfat: handle wrong stream entry size in exfat_readdir()")

Signed-off-by: Yuezhang Mo <Yuezhang.Mo@sony.com>
Reviewed-by: Andy Wu <Andy.Wu@sony.com>
Reviewed-by: Aoyama Wataru <wataru.aoyama@sony.com>
---
 fs/exfat/dir.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.25.1
  

Comments

Sungjong Seo Dec. 29, 2022, 2:28 a.m. UTC | #1
> Since seekdir() does not check whether the position is valid, the
> position may exceed the size of the directory. We found that for
> a directory with discontinuous clusters, if the position exceeds
> the size of the directory and the excess size is greater than or
> equal to the cluster size, exfat_readdir() will return -EIO,
> causing a file system error and making the file system unavailable.
> 
> Reproduce this bug by:
> 
> seekdir(dir, dir_size + cluster_size);
> dirent = readdir(dir);
> 
> The following log will be printed if mount with 'errors=remount-ro'.
> 
> [11166.712896] exFAT-fs (sdb1): error, invalid access to FAT (entry
> 0xffffffff)
> [11166.712905] exFAT-fs (sdb1): Filesystem has been set read-only
> 
> Fixes: 1e5654de0f51 ("exfat: handle wrong stream entry size in
> exfat_readdir()")
> 
> Signed-off-by: Yuezhang Mo <Yuezhang.Mo@sony.com>
> Reviewed-by: Andy Wu <Andy.Wu@sony.com>
> Reviewed-by: Aoyama Wataru <wataru.aoyama@sony.com>

Looks good. Thanks.

Reviewed-by: Sungjong Seo <sj1557.seo@samsung.com>

> ---
>  fs/exfat/dir.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/exfat/dir.c b/fs/exfat/dir.c
> index 1122bee3b634..158427e8124e 100644
> --- a/fs/exfat/dir.c
> +++ b/fs/exfat/dir.c
> @@ -100,7 +100,7 @@ static int exfat_readdir(struct inode *inode, loff_t
> *cpos, struct exfat_dir_ent
>  			clu.dir = ei->hint_bmap.clu;
>  		}
> 
> -		while (clu_offset > 0) {
> +		while (clu_offset > 0 && clu.dir != EXFAT_EOF_CLUSTER) {
>  			if (exfat_get_next_cluster(sb, &(clu.dir)))
>  				return -EIO;
> 
> --
> 2.25.1
  
Namjae Jeon Dec. 30, 2022, 1:32 a.m. UTC | #2
2022-12-26 16:23 GMT+09:00, Yuezhang.Mo@sony.com <Yuezhang.Mo@sony.com>:
> Since seekdir() does not check whether the position is valid, the
> position may exceed the size of the directory. We found that for
> a directory with discontinuous clusters, if the position exceeds
> the size of the directory and the excess size is greater than or
> equal to the cluster size, exfat_readdir() will return -EIO,
> causing a file system error and making the file system unavailable.
>
> Reproduce this bug by:
>
> seekdir(dir, dir_size + cluster_size);
> dirent = readdir(dir);
>
> The following log will be printed if mount with 'errors=remount-ro'.
>
> [11166.712896] exFAT-fs (sdb1): error, invalid access to FAT (entry
> 0xffffffff)
> [11166.712905] exFAT-fs (sdb1): Filesystem has been set read-only
>
> Fixes: 1e5654de0f51 ("exfat: handle wrong stream entry size in
> exfat_readdir()")
>
> Signed-off-by: Yuezhang Mo <Yuezhang.Mo@sony.com>
> Reviewed-by: Andy Wu <Andy.Wu@sony.com>
> Reviewed-by: Aoyama Wataru <wataru.aoyama@sony.com>
Applied, Thanks for your patch!
  

Patch

diff --git a/fs/exfat/dir.c b/fs/exfat/dir.c
index 1122bee3b634..158427e8124e 100644
--- a/fs/exfat/dir.c
+++ b/fs/exfat/dir.c
@@ -100,7 +100,7 @@  static int exfat_readdir(struct inode *inode, loff_t *cpos, struct exfat_dir_ent
 			clu.dir = ei->hint_bmap.clu;
 		}
 
-		while (clu_offset > 0) {
+		while (clu_offset > 0 && clu.dir != EXFAT_EOF_CLUSTER) {
 			if (exfat_get_next_cluster(sb, &(clu.dir)))
 				return -EIO;