[v2] ext2: Handle fiemap on empty files to prevent EINVAL

[PATCH v2] ext2: Handle fiemap on empty files to prevent EINVAL

Posted by Wei Gao 7 months, 4 weeks ago

Previously, ext2_fiemap would unconditionally apply "len = min_t(u64, len,
i_size_read(inode));", When inode->i_size was 0 (for an empty file), this
would reduce the requested len to 0. Passing len = 0 to iomap_fiemap could
then result in an -EINVAL error, even for valid queries on empty files.

Link: https://github.com/linux-test-project/ltp/issues/1246
Signed-off-by: Wei Gao <wegao@suse.com>
---
 fs/ext2/inode.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
index 30f8201c155f..591db2b4390a 100644
--- a/fs/ext2/inode.c
+++ b/fs/ext2/inode.c
@@ -895,9 +895,15 @@ int ext2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
 		u64 start, u64 len)
 {
 	int ret;
+	u64 i_size;
 
 	inode_lock(inode);
-	len = min_t(u64, len, i_size_read(inode));
+
+	i_size = i_size_read(inode);
+
+	if (i_size > 0)
+		len = min_t(u64, len, i_size_read(inode));
+
 	ret = iomap_fiemap(inode, fieinfo, start, len, &ext2_iomap_ops);
 	inode_unlock(inode);
 
-- 
2.49.0

Re: [PATCH v2] ext2: Handle fiemap on empty files to prevent EINVAL

Posted by Jan Kara 7 months, 4 weeks ago

On Fri 13-06-25 11:18:38, Wei Gao wrote:
> Previously, ext2_fiemap would unconditionally apply "len = min_t(u64, len,
> i_size_read(inode));", When inode->i_size was 0 (for an empty file), this
> would reduce the requested len to 0. Passing len = 0 to iomap_fiemap could
> then result in an -EINVAL error, even for valid queries on empty files.
> 
> Link: https://github.com/linux-test-project/ltp/issues/1246
> Signed-off-by: Wei Gao <wegao@suse.com>

...

> diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
> index 30f8201c155f..591db2b4390a 100644
> --- a/fs/ext2/inode.c
> +++ b/fs/ext2/inode.c
> @@ -895,9 +895,15 @@ int ext2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>  		u64 start, u64 len)
>  {
>  	int ret;
> +	u64 i_size;
>  
>  	inode_lock(inode);
> -	len = min_t(u64, len, i_size_read(inode));
> +
> +	i_size = i_size_read(inode);
> +
> +	if (i_size > 0)
> +		len = min_t(u64, len, i_size_read(inode));


Thanks! This would actually lead to excessively slow fiemap for 0-length
files. So what I've ended up with is attached modification of your patch.

> +
>  	ret = iomap_fiemap(inode, fieinfo, start, len, &ext2_iomap_ops);
>  	inode_unlock(inode);
>  
> -- 
> 2.49.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR
---

From a099b09a3342a0b28ea330e405501b5b4d0424b4 Mon Sep 17 00:00:00 2001
From: Wei Gao <wegao@suse.com>
Date: Fri, 13 Jun 2025 11:18:38 -0400
Subject: [PATCH] ext2: Handle fiemap on empty files to prevent EINVAL

Previously, ext2_fiemap would unconditionally apply "len = min_t(u64, len,
i_size_read(inode));", When inode->i_size was 0 (for an empty file), this
would reduce the requested len to 0. Passing len = 0 to iomap_fiemap could
then result in an -EINVAL error, even for valid queries on empty files.

Link: https://github.com/linux-test-project/ltp/issues/1246
Signed-off-by: Wei Gao <wegao@suse.com>
Signed-off-by: Jan Kara <jack@suse.cz>
Link: https://patch.msgid.link/20250613152402.3432135-1-wegao@suse.com
---
 fs/ext2/inode.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
index 30f8201c155f..177b1f852b63 100644
--- a/fs/ext2/inode.c
+++ b/fs/ext2/inode.c
@@ -895,9 +895,19 @@ int ext2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
 		u64 start, u64 len)
 {
 	int ret;
+	loff_t i_size;
 
 	inode_lock(inode);
-	len = min_t(u64, len, i_size_read(inode));
+	i_size = i_size_read(inode);
+	/*
+	 * iomap_fiemap() returns EINVAL for 0 length. Make sure we don't trim
+	 * length to 0 but still trim the range as much as possible since
+	 * ext2_get_blocks() iterates unmapped space block by block which is
+	 * slow.
+	 */
+	if (i_size == 0)
+		i_size = 1;
+	len = min_t(u64, len, i_size);
 	ret = iomap_fiemap(inode, fieinfo, start, len, &ext2_iomap_ops);
 	inode_unlock(inode);
 
-- 
2.43.0

Re: [PATCH v2] ext2: Handle fiemap on empty files to prevent EINVAL

Posted by Wei Gao 7 months, 4 weeks ago

On Fri, Jun 13, 2025 at 11:42:17AM +0200, Jan Kara wrote:
> On Fri 13-06-25 11:18:38, Wei Gao wrote:
> > Previously, ext2_fiemap would unconditionally apply "len = min_t(u64, len,
> > i_size_read(inode));", When inode->i_size was 0 (for an empty file), this
> > would reduce the requested len to 0. Passing len = 0 to iomap_fiemap could
> > then result in an -EINVAL error, even for valid queries on empty files.
> > 
> > Link: https://github.com/linux-test-project/ltp/issues/1246
> > Signed-off-by: Wei Gao <wegao@suse.com>
> 
> ...
> 
> > diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
> > index 30f8201c155f..591db2b4390a 100644
> > --- a/fs/ext2/inode.c
> > +++ b/fs/ext2/inode.c
> > @@ -895,9 +895,15 @@ int ext2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> >  		u64 start, u64 len)
> >  {
> >  	int ret;
> > +	u64 i_size;
> >  
> >  	inode_lock(inode);
> > -	len = min_t(u64, len, i_size_read(inode));
> > +
> > +	i_size = i_size_read(inode);
> > +
> > +	if (i_size > 0)
> > +		len = min_t(u64, len, i_size_read(inode));
> 
> 
> Thanks! This would actually lead to excessively slow fiemap for 0-length
> files. So what I've ended up with is attached modification of your patch.
Thank you for your patient review, I really appreciate it. 

BTW i have stupid question:
Where can I see the real-time status of this patch? such as whether it has been merged?
I have checked https://patchwork.kernel.org/project/linux-fsdevel/list/
but do not find current patch, maybe this patch need specific sent it to
linux-fsdevel@vger.kernel.org? I just get maillist through scripts/get_maintainer.pl but
mail list not contain linux-fsdevel@vger.kernel.org.

> 
> > +
> >  	ret = iomap_fiemap(inode, fieinfo, start, len, &ext2_iomap_ops);
> >  	inode_unlock(inode);
> >  
> > -- 
> > 2.49.0
> > 
> -- 
> Jan Kara <jack@suse.com>
> SUSE Labs, CR
> ---
> 
> From a099b09a3342a0b28ea330e405501b5b4d0424b4 Mon Sep 17 00:00:00 2001
> From: Wei Gao <wegao@suse.com>
> Date: Fri, 13 Jun 2025 11:18:38 -0400
> Subject: [PATCH] ext2: Handle fiemap on empty files to prevent EINVAL
> 
> Previously, ext2_fiemap would unconditionally apply "len = min_t(u64, len,
> i_size_read(inode));", When inode->i_size was 0 (for an empty file), this
> would reduce the requested len to 0. Passing len = 0 to iomap_fiemap could
> then result in an -EINVAL error, even for valid queries on empty files.
> 
> Link: https://github.com/linux-test-project/ltp/issues/1246
> Signed-off-by: Wei Gao <wegao@suse.com>
> Signed-off-by: Jan Kara <jack@suse.cz>
> Link: https://patch.msgid.link/20250613152402.3432135-1-wegao@suse.com
> ---
>  fs/ext2/inode.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
> index 30f8201c155f..177b1f852b63 100644
> --- a/fs/ext2/inode.c
> +++ b/fs/ext2/inode.c
> @@ -895,9 +895,19 @@ int ext2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>  		u64 start, u64 len)
>  {
>  	int ret;
> +	loff_t i_size;
>  
>  	inode_lock(inode);
> -	len = min_t(u64, len, i_size_read(inode));
> +	i_size = i_size_read(inode);
> +	/*
> +	 * iomap_fiemap() returns EINVAL for 0 length. Make sure we don't trim
> +	 * length to 0 but still trim the range as much as possible since
> +	 * ext2_get_blocks() iterates unmapped space block by block which is
> +	 * slow.
> +	 */
> +	if (i_size == 0)
> +		i_size = 1;
> +	len = min_t(u64, len, i_size);
>  	ret = iomap_fiemap(inode, fieinfo, start, len, &ext2_iomap_ops);
>  	inode_unlock(inode);
>  
> -- 
> 2.43.0
>

Re: [PATCH v2] ext2: Handle fiemap on empty files to prevent EINVAL

Posted by Jan Kara 7 months, 4 weeks ago

On Fri 13-06-25 18:59:05, Wei Gao wrote:
> On Fri, Jun 13, 2025 at 11:42:17AM +0200, Jan Kara wrote:
> > On Fri 13-06-25 11:18:38, Wei Gao wrote:
> > > Previously, ext2_fiemap would unconditionally apply "len = min_t(u64, len,
> > > i_size_read(inode));", When inode->i_size was 0 (for an empty file), this
> > > would reduce the requested len to 0. Passing len = 0 to iomap_fiemap could
> > > then result in an -EINVAL error, even for valid queries on empty files.
> > > 
> > > Link: https://github.com/linux-test-project/ltp/issues/1246
> > > Signed-off-by: Wei Gao <wegao@suse.com>
> > 
> > ...
> > 
> > > diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
> > > index 30f8201c155f..591db2b4390a 100644
> > > --- a/fs/ext2/inode.c
> > > +++ b/fs/ext2/inode.c
> > > @@ -895,9 +895,15 @@ int ext2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> > >  		u64 start, u64 len)
> > >  {
> > >  	int ret;
> > > +	u64 i_size;
> > >  
> > >  	inode_lock(inode);
> > > -	len = min_t(u64, len, i_size_read(inode));
> > > +
> > > +	i_size = i_size_read(inode);
> > > +
> > > +	if (i_size > 0)
> > > +		len = min_t(u64, len, i_size_read(inode));
> > 
> > 
> > Thanks! This would actually lead to excessively slow fiemap for 0-length
> > files. So what I've ended up with is attached modification of your patch.
> Thank you for your patient review, I really appreciate it. 
> 
> BTW i have stupid question:
> Where can I see the real-time status of this patch? such as whether it has been merged?
> I have checked https://patchwork.kernel.org/project/linux-fsdevel/list/
> but do not find current patch, maybe this patch need specific sent it to
> linux-fsdevel@vger.kernel.org? I just get maillist through scripts/get_maintainer.pl but
> mail list not contain linux-fsdevel@vger.kernel.org.

You cannot easily check it. You can see the patch is sitting in
git.kernel.org/pub/scm/linux/kernel/git/jack/linux-fs.git for_next
branch. During the next merge window, I'll push it to Linus.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR