mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] f2fs: disallow mmap write and data-modifying fallocate on atomic files
@ 2026-10-02 17:17 Daeho Jeong
  2026-10-09  8:35 ` [f2fs-dev] " Chao Yu
  0 siblings, 1 reply; 4+ messages in thread
From: Daeho Jeong @ 2026-10-02 17:17 UTC (permalink / raw)
  To: linux-kernel, linux-f2fs-devel, kernel-team; +Cc: Daeho Jeong

From: Daeho Jeong <daehojeong@google.com>

Stores through a shared writable mapping and fallocate with
FALLOC_FL_PUNCH_HOLE, FALLOC_FL_ZERO_RANGE, FALLOC_FL_COLLAPSE_RANGE or
FALLOC_FL_INSERT_RANGE do not go through ->write_begin, so they bypass
the COW inode and modify the original inode directly. Reject them while
the file is in atomic write mode, as fallocate already does for pinned
and compressed files. A write fault gets SIGBUS and fallocate gets
-EOPNOTSUPP.

fallocate without these flags only preallocates blocks and stays
allowed.

Fixes: 3db1de0e582c ("f2fs: change the current atomic write way")
Signed-off-by: Daeho Jeong <daehojeong@google.com>
---
 fs/f2fs/file.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index ef4d218e694..56c686b8580 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -140,6 +140,10 @@ static vm_fault_t f2fs_vm_page_mkwrite(struct vm_fault *vmf)
 		return VM_FAULT_SIGBUS;
 	}
 
+	/* mmap stores bypass the COW inode and would break atomicity */
+	if (f2fs_is_atomic_file(inode))
+		return VM_FAULT_SIGBUS;
+
 	if (is_inode_flag_set(inode, FI_COMPRESS_RELEASED)) {
 		err = -EIO;
 		goto out;
@@ -2113,9 +2117,11 @@ static long f2fs_fallocate(struct file *file, int mode,
 
 	/*
 	 * Pinned file should not support partial truncation since the block
-	 * can be used by applications.
+	 * can be used by applications. Atomic files should not either, since
+	 * these modify the original inode directly and break atomicity.
 	 */
-	if ((f2fs_compressed_file(inode) || f2fs_is_pinned_file(inode)) &&
+	if ((f2fs_compressed_file(inode) || f2fs_is_pinned_file(inode) ||
+	     f2fs_is_atomic_file(inode)) &&
 		(mode & (FALLOC_FL_PUNCH_HOLE | FALLOC_FL_COLLAPSE_RANGE |
 			FALLOC_FL_ZERO_RANGE | FALLOC_FL_INSERT_RANGE))) {
 		ret = -EOPNOTSUPP;
-- 
2.56.0.rc1.315.gc6ed9934b7-goog


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [f2fs-dev] [PATCH] f2fs: disallow mmap write and data-modifying fallocate on atomic files
  2026-10-02 17:17 [PATCH] f2fs: disallow mmap write and data-modifying fallocate on atomic files Daeho Jeong
@ 2026-10-09  8:35 ` Chao Yu
  2026-10-09 15:05   ` Daeho Jeong
  0 siblings, 1 reply; 4+ messages in thread
From: Chao Yu @ 2026-10-09  8:35 UTC (permalink / raw)
  To: Daeho Jeong, linux-kernel, linux-f2fs-devel, kernel-team
  Cc: chao, Daeho Jeong

On 10/3/26 01:17, Daeho Jeong wrote:
> From: Daeho Jeong <daehojeong@google.com>
> 
> Stores through a shared writable mapping and fallocate with
> FALLOC_FL_PUNCH_HOLE, FALLOC_FL_ZERO_RANGE, FALLOC_FL_COLLAPSE_RANGE or
> FALLOC_FL_INSERT_RANGE do not go through ->write_begin, so they bypass
> the COW inode and modify the original inode directly. Reject them while
> the file is in atomic write mode, as fallocate already does for pinned
> and compressed files. A write fault gets SIGBUS and fallocate gets
> -EOPNOTSUPP.
> 
> fallocate without these flags only preallocates blocks and stays
> allowed.
> 
> Fixes: 3db1de0e582c ("f2fs: change the current atomic write way")
> Signed-off-by: Daeho Jeong <daehojeong@google.com>
> ---
>  fs/f2fs/file.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index ef4d218e694..56c686b8580 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -140,6 +140,10 @@ static vm_fault_t f2fs_vm_page_mkwrite(struct vm_fault *vmf)
>  		return VM_FAULT_SIGBUS;
>  	}
>  
> +	/* mmap stores bypass the COW inode and would break atomicity */
> +	if (f2fs_is_atomic_file(inode))
> +		return VM_FAULT_SIGBUS;

Will we suffer race issue due to the check w/o lock?

Thanks,

> +
>  	if (is_inode_flag_set(inode, FI_COMPRESS_RELEASED)) {
>  		err = -EIO;
>  		goto out;
> @@ -2113,9 +2117,11 @@ static long f2fs_fallocate(struct file *file, int mode,
>  
>  	/*
>  	 * Pinned file should not support partial truncation since the block
> -	 * can be used by applications.
> +	 * can be used by applications. Atomic files should not either, since
> +	 * these modify the original inode directly and break atomicity.
>  	 */
> -	if ((f2fs_compressed_file(inode) || f2fs_is_pinned_file(inode)) &&
> +	if ((f2fs_compressed_file(inode) || f2fs_is_pinned_file(inode) ||
> +	     f2fs_is_atomic_file(inode)) &&
>  		(mode & (FALLOC_FL_PUNCH_HOLE | FALLOC_FL_COLLAPSE_RANGE |
>  			FALLOC_FL_ZERO_RANGE | FALLOC_FL_INSERT_RANGE))) {
>  		ret = -EOPNOTSUPP;


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [f2fs-dev] [PATCH] f2fs: disallow mmap write and data-modifying fallocate on atomic files
  2026-10-09  8:35 ` [f2fs-dev] " Chao Yu
@ 2026-10-09 15:05   ` Daeho Jeong
  2026-10-10  1:40     ` Chao Yu
  0 siblings, 1 reply; 4+ messages in thread
From: Daeho Jeong @ 2026-10-09 15:05 UTC (permalink / raw)
  To: Chao Yu; +Cc: linux-kernel, linux-f2fs-devel, kernel-team, Daeho Jeong

On Fri, Oct 9, 2026 at 1:35 AM Chao Yu <chao@kernel.org> wrote:
>
> On 10/3/26 01:17, Daeho Jeong wrote:
> > From: Daeho Jeong <daehojeong@google.com>
> >
> > Stores through a shared writable mapping and fallocate with
> > FALLOC_FL_PUNCH_HOLE, FALLOC_FL_ZERO_RANGE, FALLOC_FL_COLLAPSE_RANGE or
> > FALLOC_FL_INSERT_RANGE do not go through ->write_begin, so they bypass
> > the COW inode and modify the original inode directly. Reject them while
> > the file is in atomic write mode, as fallocate already does for pinned
> > and compressed files. A write fault gets SIGBUS and fallocate gets
> > -EOPNOTSUPP.
> >
> > fallocate without these flags only preallocates blocks and stays
> > allowed.
> >
> > Fixes: 3db1de0e582c ("f2fs: change the current atomic write way")
> > Signed-off-by: Daeho Jeong <daehojeong@google.com>
> > ---
> >  fs/f2fs/file.c | 10 ++++++++--
> >  1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> > index ef4d218e694..56c686b8580 100644
> > --- a/fs/f2fs/file.c
> > +++ b/fs/f2fs/file.c
> > @@ -140,6 +140,10 @@ static vm_fault_t f2fs_vm_page_mkwrite(struct vm_fault *vmf)
> >               return VM_FAULT_SIGBUS;
> >       }
> >
> > +     /* mmap stores bypass the COW inode and would break atomicity */
> > +     if (f2fs_is_atomic_file(inode))
> > +             return VM_FAULT_SIGBUS;
>
> Will we suffer race issue due to the check w/o lock?
>
> Thanks,

Hi Chao,

Yes, there is a race with f2fs_ioc_start_atomic_write().

page_mkwrite cannot take inode_lock, because the lock order is i_rwsem
-> mmap_lock (e.g. fiemap copies to user under inode_lock). So v2 will
use invalidate_lock instead:

f2fs_ioc_start_atomic_write() holds filemap_invalidate_lock() from the
flush until FI_ATOMIC_FILE is set.
f2fs_vm_page_mkwrite() rechecks the flag after
filemap_invalidate_lock_shared(), which it holds until the folio is
dirtied.

If you are okay with this, I'll send v2.

Thanks,

>
> > +
> >       if (is_inode_flag_set(inode, FI_COMPRESS_RELEASED)) {
> >               err = -EIO;
> >               goto out;
> > @@ -2113,9 +2117,11 @@ static long f2fs_fallocate(struct file *file, int mode,
> >
> >       /*
> >        * Pinned file should not support partial truncation since the block
> > -      * can be used by applications.
> > +      * can be used by applications. Atomic files should not either, since
> > +      * these modify the original inode directly and break atomicity.
> >        */
> > -     if ((f2fs_compressed_file(inode) || f2fs_is_pinned_file(inode)) &&
> > +     if ((f2fs_compressed_file(inode) || f2fs_is_pinned_file(inode) ||
> > +          f2fs_is_atomic_file(inode)) &&
> >               (mode & (FALLOC_FL_PUNCH_HOLE | FALLOC_FL_COLLAPSE_RANGE |
> >                       FALLOC_FL_ZERO_RANGE | FALLOC_FL_INSERT_RANGE))) {
> >               ret = -EOPNOTSUPP;
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [f2fs-dev] [PATCH] f2fs: disallow mmap write and data-modifying fallocate on atomic files
  2026-10-09 15:05   ` Daeho Jeong
@ 2026-10-10  1:40     ` Chao Yu
  0 siblings, 0 replies; 4+ messages in thread
From: Chao Yu @ 2026-10-10  1:40 UTC (permalink / raw)
  To: Daeho Jeong
  Cc: chao, linux-kernel, linux-f2fs-devel, kernel-team, Daeho Jeong



On 10/9/26 23:05, Daeho Jeong wrote:
> On Fri, Oct 9, 2026 at 1:35 AM Chao Yu <chao@kernel.org> wrote:
>>
>> On 10/3/26 01:17, Daeho Jeong wrote:
>>> From: Daeho Jeong <daehojeong@google.com>
>>>
>>> Stores through a shared writable mapping and fallocate with
>>> FALLOC_FL_PUNCH_HOLE, FALLOC_FL_ZERO_RANGE, FALLOC_FL_COLLAPSE_RANGE or
>>> FALLOC_FL_INSERT_RANGE do not go through ->write_begin, so they bypass
>>> the COW inode and modify the original inode directly. Reject them while
>>> the file is in atomic write mode, as fallocate already does for pinned
>>> and compressed files. A write fault gets SIGBUS and fallocate gets
>>> -EOPNOTSUPP.
>>>
>>> fallocate without these flags only preallocates blocks and stays
>>> allowed.
>>>
>>> Fixes: 3db1de0e582c ("f2fs: change the current atomic write way")
>>> Signed-off-by: Daeho Jeong <daehojeong@google.com>
>>> ---
>>>  fs/f2fs/file.c | 10 ++++++++--
>>>  1 file changed, 8 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
>>> index ef4d218e694..56c686b8580 100644
>>> --- a/fs/f2fs/file.c
>>> +++ b/fs/f2fs/file.c
>>> @@ -140,6 +140,10 @@ static vm_fault_t f2fs_vm_page_mkwrite(struct vm_fault *vmf)
>>>               return VM_FAULT_SIGBUS;
>>>       }
>>>
>>> +     /* mmap stores bypass the COW inode and would break atomicity */
>>> +     if (f2fs_is_atomic_file(inode))
>>> +             return VM_FAULT_SIGBUS;
>>
>> Will we suffer race issue due to the check w/o lock?
>>
>> Thanks,
> 
> Hi Chao,
> 
> Yes, there is a race with f2fs_ioc_start_atomic_write().
> 
> page_mkwrite cannot take inode_lock, because the lock order is i_rwsem
> -> mmap_lock (e.g. fiemap copies to user under inode_lock). So v2 will
> use invalidate_lock instead:
> 
> f2fs_ioc_start_atomic_write() holds filemap_invalidate_lock() from the
> flush until FI_ATOMIC_FILE is set.
> f2fs_vm_page_mkwrite() rechecks the flag after
> filemap_invalidate_lock_shared(), which it holds until the folio is
> dirtied.
> 
> If you are okay with this, I'll send v2.

Daeho, looks fine to me, please go ahead. :)

Thanks,

> 
> Thanks,
> 
>>
>>> +
>>>       if (is_inode_flag_set(inode, FI_COMPRESS_RELEASED)) {
>>>               err = -EIO;
>>>               goto out;
>>> @@ -2113,9 +2117,11 @@ static long f2fs_fallocate(struct file *file, int mode,
>>>
>>>       /*
>>>        * Pinned file should not support partial truncation since the block
>>> -      * can be used by applications.
>>> +      * can be used by applications. Atomic files should not either, since
>>> +      * these modify the original inode directly and break atomicity.
>>>        */
>>> -     if ((f2fs_compressed_file(inode) || f2fs_is_pinned_file(inode)) &&
>>> +     if ((f2fs_compressed_file(inode) || f2fs_is_pinned_file(inode) ||
>>> +          f2fs_is_atomic_file(inode)) &&
>>>               (mode & (FALLOC_FL_PUNCH_HOLE | FALLOC_FL_COLLAPSE_RANGE |
>>>                       FALLOC_FL_ZERO_RANGE | FALLOC_FL_INSERT_RANGE))) {
>>>               ret = -EOPNOTSUPP;
>>


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-10-10  1:40 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-02 17:17 [PATCH] f2fs: disallow mmap write and data-modifying fallocate on atomic files Daeho Jeong
2026-10-09  8:35 ` [f2fs-dev] " Chao Yu
2026-10-09 15:05   ` Daeho Jeong
2026-10-10  1:40     ` Chao Yu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®