[PATCH 0/1] exfat: fix i_blocks for files truncated over 4 GiB

[PATCH 0/1] exfat: fix i_blocks for files truncated over 4 GiB

[Christophe Vu-Brugier]

From: Christophe Vu-Brugier <christophe.vu-brugier@seagate.com>

The following patch fixes an issue in the exFAT driver. The number of
allocated blocks becomes wrong if a file is truncated over 4 GiB.

A similar issue was fixed last month by Sungjong Seo:

  commit 0c336d6e33f4 ("exfat: fix incorrect loading of i_blocks for
                        large files")

Below is a test case for the issue. A 7 GiB file is truncated to 5 GiB
but stat() st_blocks shows only 1 GiB being used.

$ dd if=/dev/urandom of=file.bin bs=1024k count=7168

$ /sbin/xfs_io -c "stat" file.bin
fd.path = "file.bin"
fd.flags = non-sync,non-direct,read-write
stat.ino = 11
stat.type = regular file
stat.size = 7516192768
stat.blocks = 14680064

$ /sbin/xfs_io -c "truncate 5368709120" file.bin

$ /sbin/xfs_io -c "stat" file.bin
fd.path = "file.bin"
fd.flags = non-sync,non-direct,read-write
stat.ino = 11
stat.type = regular file
stat.size = 5368709120
stat.blocks =  2097152

Christophe Vu-Brugier (1):
  exfat: fix i_blocks for files truncated over 4 GiB

 fs/exfat/file.c  | 2 +-
 fs/exfat/super.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

-- 
2.33.0

[PATCH 1/1] exfat: fix i_blocks for files truncated over 4 GiB

[Christophe Vu-Brugier]

From: Christophe Vu-Brugier <christophe.vu-brugier@seagate.com>

In exfat_truncate(), the computation of inode->i_blocks is wrong if
the file is larger than 4 GiB because a 32-bit variable is used as a
mask. This is fixed by casting the variable to loff_t which is 64-bit.

Also fix the same computation in exfat_read_root().

This commit is similar to Sungjong Seo's fix in exfat_fill_inode()
last month:

  commit 0c336d6e33f4 ("exfat: fix incorrect loading of i_blocks for
                        large files")

Signed-off-by: Christophe Vu-Brugier <christophe.vu-brugier@seagate.com>
---
 fs/exfat/file.c  | 2 +-
 fs/exfat/super.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/exfat/file.c b/fs/exfat/file.c
index 6af0191b648f..109ade79da33 100644
--- a/fs/exfat/file.c
+++ b/fs/exfat/file.c
@@ -252,7 +252,7 @@ void exfat_truncate(struct inode *inode, loff_t size)
 		mark_inode_dirty(inode);
 
 	inode->i_blocks = ((i_size_read(inode) + (sbi->cluster_size - 1)) &
-			~(sbi->cluster_size - 1)) >> inode->i_blkbits;
+		~((loff_t)sbi->cluster_size - 1)) >> inode->i_blkbits;
 write_size:
 	aligned_size = i_size_read(inode);
 	if (aligned_size & (blocksize - 1)) {
diff --git a/fs/exfat/super.c b/fs/exfat/super.c
index 5539ffc20d16..ea16769380c6 100644
--- a/fs/exfat/super.c
+++ b/fs/exfat/super.c
@@ -364,8 +364,8 @@ static int exfat_read_root(struct inode *inode)
 	inode->i_op = &exfat_dir_inode_operations;
 	inode->i_fop = &exfat_dir_operations;
 
-	inode->i_blocks = ((i_size_read(inode) + (sbi->cluster_size - 1))
-			& ~(sbi->cluster_size - 1)) >> inode->i_blkbits;
+	inode->i_blocks = ((i_size_read(inode) + (sbi->cluster_size - 1)) &
+		~((loff_t)sbi->cluster_size - 1)) >> inode->i_blkbits;
 	EXFAT_I(inode)->i_pos = ((loff_t)sbi->root_dir << 32) | 0xffffffff;
 	EXFAT_I(inode)->i_size_aligned = i_size_read(inode);
 	EXFAT_I(inode)->i_size_ondisk = i_size_read(inode);
-- 
2.33.0

Re: [PATCH 1/1] exfat: fix i_blocks for files truncated over 4 GiB

[Matthew Wilcox]

On Thu, Nov 18, 2021 at 10:28:28PM +0100, Christophe Vu-Brugier wrote:
>  	inode->i_blocks = ((i_size_read(inode) + (sbi->cluster_size - 1)) &
> -			~(sbi->cluster_size - 1)) >> inode->i_blkbits;
> +		~((loff_t)sbi->cluster_size - 1)) >> inode->i_blkbits;

Isn't this a convoluted way to write:

	inode->i_blocks = round_up(i_size_read(inode), sbi->cluster_size) >>
				inode->i_blkbits;
?

Re: [PATCH 1/1] exfat: fix i_blocks for files truncated over 4 GiB

[Christophe Vu-Brugier]

Hi Matthew,

Le 18/11/2021 à 22:50, Matthew Wilcox a écrit :
> On Thu, Nov 18, 2021 at 10:28:28PM +0100, Christophe Vu-Brugier wrote:
>>   	inode->i_blocks = ((i_size_read(inode) + (sbi->cluster_size - 1)) &
>> -			~(sbi->cluster_size - 1)) >> inode->i_blkbits;
>> +		~((loff_t)sbi->cluster_size - 1)) >> inode->i_blkbits;
> 
> Isn't this a convoluted way to write:
> 
> 	inode->i_blocks = round_up(i_size_read(inode), sbi->cluster_size) >>
> 				inode->i_blkbits;
> ?

Yes, it is. And we have evidence that it is more error prone.

I will update my patch with what you suggest.

Thanks!

-- 
Christophe Vu-Brugier

[PATCH v2] exfat: fix i_blocks for files truncated over 4 GiB

[Christophe Vu-Brugier]

From: Christophe Vu-Brugier <christophe.vu-brugier@seagate.com>

In exfat_truncate(), the computation of inode->i_blocks is wrong if
the file is larger than 4 GiB because a 32-bit variable is used as a
mask. This is fixed and simplified by using round_up().

Also fix the same buggy computation in exfat_read_root() and another
(correct) one in exfat_fill_inode(). The latter was fixed another way
last month but can be simplified by using round_up() as well. See:

  commit 0c336d6e33f4 ("exfat: fix incorrect loading of i_blocks for
                        large files")

Signed-off-by: Christophe Vu-Brugier <christophe.vu-brugier@seagate.com>
Suggested-by: Matthew Wilcox <willy@infradead.org>
---
 fs/exfat/file.c  | 4 ++--
 fs/exfat/inode.c | 4 ++--
 fs/exfat/super.c | 4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/fs/exfat/file.c b/fs/exfat/file.c
index 6af0191b648f..3dafd3c013d7 100644
--- a/fs/exfat/file.c
+++ b/fs/exfat/file.c
@@ -251,8 +251,8 @@ void exfat_truncate(struct inode *inode, loff_t size)
 	else
 		mark_inode_dirty(inode);
 
-	inode->i_blocks = ((i_size_read(inode) + (sbi->cluster_size - 1)) &
-			~(sbi->cluster_size - 1)) >> inode->i_blkbits;
+	inode->i_blocks = round_up(i_size_read(inode), sbi->cluster_size) >>
+				inode->i_blkbits;
 write_size:
 	aligned_size = i_size_read(inode);
 	if (aligned_size & (blocksize - 1)) {
diff --git a/fs/exfat/inode.c b/fs/exfat/inode.c
index 1c7aa1ea4724..464282376483 100644
--- a/fs/exfat/inode.c
+++ b/fs/exfat/inode.c
@@ -603,8 +603,8 @@ static int exfat_fill_inode(struct inode *inode, struct exfat_dir_entry *info)
 
 	exfat_save_attr(inode, info->attr);
 
-	inode->i_blocks = ((i_size_read(inode) + (sbi->cluster_size - 1)) &
-		~((loff_t)sbi->cluster_size - 1)) >> inode->i_blkbits;
+	inode->i_blocks = round_up(i_size_read(inode), sbi->cluster_size) >>
+				inode->i_blkbits;
 	inode->i_mtime = info->mtime;
 	inode->i_ctime = info->mtime;
 	ei->i_crtime = info->crtime;
diff --git a/fs/exfat/super.c b/fs/exfat/super.c
index 5539ffc20d16..bd04c8b25b59 100644
--- a/fs/exfat/super.c
+++ b/fs/exfat/super.c
@@ -364,8 +364,8 @@ static int exfat_read_root(struct inode *inode)
 	inode->i_op = &exfat_dir_inode_operations;
 	inode->i_fop = &exfat_dir_operations;
 
-	inode->i_blocks = ((i_size_read(inode) + (sbi->cluster_size - 1))
-			& ~(sbi->cluster_size - 1)) >> inode->i_blkbits;
+	inode->i_blocks = round_up(i_size_read(inode), sbi->cluster_size) >>
+				inode->i_blkbits;
 	EXFAT_I(inode)->i_pos = ((loff_t)sbi->root_dir << 32) | 0xffffffff;
 	EXFAT_I(inode)->i_size_aligned = i_size_read(inode);
 	EXFAT_I(inode)->i_size_ondisk = i_size_read(inode);
-- 
2.33.0

RE: [PATCH v2] exfat: fix i_blocks for files truncated over 4 GiB

[Sungjong Seo]

> From: Christophe Vu-Brugier <christophe.vu-brugier@seagate.com>
> 
> In exfat_truncate(), the computation of inode->i_blocks is wrong if the
> file is larger than 4 GiB because a 32-bit variable is used as a mask.
> This is fixed and simplified by using round_up().
> 
> Also fix the same buggy computation in exfat_read_root() and another
> (correct) one in exfat_fill_inode(). The latter was fixed another way last
> month but can be simplified by using round_up() as well. See:
> 
>   commit 0c336d6e33f4 ("exfat: fix incorrect loading of i_blocks for
>                         large files")
> 
> Signed-off-by: Christophe Vu-Brugier <christophe.vu-brugier@seagate.com>
> Suggested-by: Matthew Wilcox <willy@infradead.org>

Thanks for your patch!
Please update your patch again with below tags.

Fixes: 719c1e1829166 ("exfat: add super block operations")
Fixes: 98d917047e8b7 ("exfat: add file operations")
Cc: stable@vger.kernel.org # v5.7+

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

Re: [PATCH v2] exfat: fix i_blocks for files truncated over 4 GiB

[Namjae Jeon]

2021-11-22 11:10 GMT+09:00, Sungjong Seo <sj1557.seo@samsung.com>:
>> From: Christophe Vu-Brugier <christophe.vu-brugier@seagate.com>
>>
>> In exfat_truncate(), the computation of inode->i_blocks is wrong if the
>> file is larger than 4 GiB because a 32-bit variable is used as a mask.
>> This is fixed and simplified by using round_up().
>>
>> Also fix the same buggy computation in exfat_read_root() and another
>> (correct) one in exfat_fill_inode(). The latter was fixed another way
>> last
>> month but can be simplified by using round_up() as well. See:
>>
>>   commit 0c336d6e33f4 ("exfat: fix incorrect loading of i_blocks for
>>                         large files")
>>
>> Signed-off-by: Christophe Vu-Brugier <christophe.vu-brugier@seagate.com>
>> Suggested-by: Matthew Wilcox <willy@infradead.org>
>
> Thanks for your patch!
> Please update your patch again with below tags.
There is no need to send a patch again.
I will directly update and apply it.

Thanks!
>
> Fixes: 719c1e1829166 ("exfat: add super block operations")
> Fixes: 98d917047e8b7 ("exfat: add file operations")
> Cc: stable@vger.kernel.org # v5.7+
>
> Reviewed-by: Sungjong Seo <sj1557.seo@samsung.com>
>
>