[PATCH] shmem: skip empty unmap ranges when punching holes

[PATCH] shmem: skip empty unmap ranges when punching holes

[Hungyu Lin]

When a hole ends within the first page, shmem_fallocate() calculates
unmap_end as -1. Casting it to u64 makes the range check succeed even
though the hole contains no complete pages.

For example, punching one byte at offset zero calls unmap_mapping_range()
with a zero length, which means unmap to the end of the file. The file
contents are preserved outside the hole, but unrelated mappings are
removed and must fault back in. This can be reproduced with fallocate -p
-o 0 -l 1 on a mapped tmpfs file.

Compare the exclusive rounded end against the rounded start to skip
empty ranges. Keep the unsigned comparison so rounding the start near
the maximum file offset continues to be handled correctly.

Fixes: 83e4fa9c16e4 ("tmpfs: support fallocate FALLOC_FL_PUNCH_HOLE")
Signed-off-by: Hungyu Lin <dennylin0707@gmail.com>
---
 mm/shmem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/shmem.c b/mm/shmem.c
index d97fc32e7c94..08d5336c27a0 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -3864,7 +3864,7 @@ static long shmem_fallocate(struct file *file, int mode, loff_t offset,
 		WRITE_ONCE(inode->i_private, &shmem_falloc);
 		spin_unlock(&inode->i_lock);
 
-		if ((u64)unmap_end > (u64)unmap_start)
+		if ((u64)(unmap_end + 1) > (u64)unmap_start)
 			unmap_mapping_range(mapping, unmap_start,
 					    1 + unmap_end - unmap_start, 0);
 		shmem_truncate_range(inode, offset, offset + len - 1);
-- 
2.43.0

Re: [PATCH] shmem: skip empty unmap ranges when punching holes

[Baolin Wang]

On 10/28/26 1:17 AM, Hungyu Lin wrote:
> When a hole ends within the first page, shmem_fallocate() calculates
> unmap_end as -1. Casting it to u64 makes the range check succeed even
> though the hole contains no complete pages.
> 
> For example, punching one byte at offset zero calls unmap_mapping_range()
> with a zero length, which means unmap to the end of the file. The file
> contents are preserved outside the hole, but unrelated mappings are
> removed and must fault back in. This can be reproduced with fallocate -p
> -o 0 -l 1 on a mapped tmpfs file.
> 
> Compare the exclusive rounded end against the rounded start to skip
> empty ranges. Keep the unsigned comparison so rounding the start near
> the maximum file offset continues to be handled correctly.
> 
> Fixes: 83e4fa9c16e4 ("tmpfs: support fallocate FALLOC_FL_PUNCH_HOLE")
> Signed-off-by: Hungyu Lin <dennylin0707@gmail.com>
> ---

LGTM.
Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>

Re: [PATCH] shmem: skip empty unmap ranges when punching holes

[David Hildenbrand (Arm)]

On 10/27/26 18:17, Hungyu Lin wrote:
> When a hole ends within the first page, shmem_fallocate() calculates
> unmap_end as -1. Casting it to u64 makes the range check succeed even
> though the hole contains no complete pages.
> 
> For example, punching one byte at offset zero calls unmap_mapping_range()
> with a zero length, which means unmap to the end of the file. The file
> contents are preserved outside the hole, but unrelated mappings are
> removed and must fault back in. This can be reproduced with fallocate -p
> -o 0 -l 1 on a mapped tmpfs file.
> 
> Compare the exclusive rounded end against the rounded start to skip
> empty ranges. Keep the unsigned comparison so rounding the start near
> the maximum file offset continues to be handled correctly.
> 
> Fixes: 83e4fa9c16e4 ("tmpfs: support fallocate FALLOC_FL_PUNCH_HOLE")

I would pick this for the next merge window. Given that this issue is ... 14
years old I don't think we need to Cc stable.

Other opinions?

> Signed-off-by: Hungyu Lin <dennylin0707@gmail.com>
> ---
>  mm/shmem.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/shmem.c b/mm/shmem.c
> index d97fc32e7c94..08d5336c27a0 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -3864,7 +3864,7 @@ static long shmem_fallocate(struct file *file, int mode, loff_t offset,
>  		WRITE_ONCE(inode->i_private, &shmem_falloc);
>  		spin_unlock(&inode->i_lock);
>  
> -		if ((u64)unmap_end > (u64)unmap_start)
> +		if ((u64)(unmap_end + 1) > (u64)unmap_start)

Can this be >= ?

>  			unmap_mapping_range(mapping, unmap_start,
>  					    1 + unmap_end - unmap_start, 0);
>  		shmem_truncate_range(inode, offset, offset + len - 1);


-- 
Cheers,

David

Re: [PATCH] shmem: skip empty unmap ranges when punching holes

[Hungyu Lin]

> Can this be >= ?

I think we should keep > here, since unmap_end + 1 is the exclusive
end. Equality would pass a zero length to unmap_mapping_range(),
which means unmap to EOF. For example, with offset = 0 and len = 1,
both sides are zero.

Thanks,
Hungyu

Re: [PATCH] shmem: skip empty unmap ranges when punching holes

[David Hildenbrand (Arm)]

On 11/3/26 20:38, Hungyu Lin wrote:
>> Can this be >= ?
> 
> I think we should keep > here, since unmap_end + 1 is the exclusive
> end. Equality would pass a zero length to unmap_mapping_range(),
> which means unmap to EOF. For example, with offset = 0 and len = 1,
> both sides are zero.

>= is much clearer than then +1 trick, no?

To be clear, I was asking whether we could do

	if ((u64)(unmap_end) >= (u64)unmap_start)

-- 
Cheers,

David

Re: [PATCH] shmem: skip empty unmap ranges when punching holes

[Hungyu Lin]

> To be clear, I was asking whether we could do
>
>         if ((u64)(unmap_end) >= (u64)unmap_start)

Sorry, I misunderstood your suggestion.

With offset = 0 and len = 1, unmap_end is -1 and unmap_start is 0.
Casting -1 to u64 makes this comparison true, so it still passes
a zero length to unmap_mapping_range(), meaning unmap to EOF.

The +1 turns -1 into 0 before the cast, so the submitted check
becomes 0 > 0 and skips the empty range.

Thanks,
Hungyu