[PATCH] coda: honor the append state of each write

[PATCH] coda: honor the append state of each write

[Hungyu Lin]

coda_open() adds O_APPEND to the container file's f_flags after it has
been opened, without updating its cached f_iocb_flags. An O_APPEND write
can therefore overwrite existing data instead of appending. For example,
writing "Z" to a file containing "abc" produces "Zbc" rather than "abcZ".

Pass RWF_APPEND or RWF_NOAPPEND to vfs_iter_write() according to the
incoming IOCB_APPEND state. This also honors changes made with F_SETFL
and per-write append overrides instead of relying on the container's
open-time flags.

Fixes: 164f4064ca81 ("keep iocb_flags() result cached in struct file")
Cc: stable@vger.kernel.org # 6.9+
Signed-off-by: Hungyu Lin <dennylin0707@gmail.com>
---
 fs/coda/file.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/coda/file.c b/fs/coda/file.c
index 6ca8bfcb58f2..ae03f0b30d84 100644
--- a/fs/coda/file.c
+++ b/fs/coda/file.c
@@ -71,6 +71,7 @@ coda_file_write_iter(struct kiocb *iocb, struct iov_iter *to)
 	struct file *host_file = cfi->cfi_container;
 	loff_t ki_pos = iocb->ki_pos;
 	size_t count = iov_iter_count(to);
+	rwf_t flags = (iocb->ki_flags & IOCB_APPEND) ? RWF_APPEND : RWF_NOAPPEND;
 	ssize_t ret;
 
 	ret = venus_access_intent(coda_inode->i_sb, coda_i2f(coda_inode),
@@ -80,7 +81,7 @@ coda_file_write_iter(struct kiocb *iocb, struct iov_iter *to)
 		goto finish_write;
 
 	inode_lock(coda_inode);
-	ret = vfs_iter_write(cfi->cfi_container, to, &iocb->ki_pos, 0);
+	ret = vfs_iter_write(host_file, to, &iocb->ki_pos, flags);
 	coda_inode->i_size = file_inode(host_file)->i_size;
 	coda_inode->i_blocks = (coda_inode->i_size + 511) >> 9;
 	inode_set_mtime_to_ts(coda_inode, inode_set_ctime_current(coda_inode));

base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
-- 
2.43.0

Re: [PATCH] coda: honor the append state of each write

[Jan Harkes]

On Wed, Nov 04, 2026 at 02:14:23PM -0500, Hungyu Lin wrote:
> coda_open() adds O_APPEND to the container file's f_flags after it has
> been opened, without updating its cached f_iocb_flags. An O_APPEND write

Why do we now have cached copies of f_flags around? That sounds like a
recipe for getting things out of sync.

Anyway, your patch subtly changes the container file we write from
cfi->cfi_container to host_file and I am not convinced that is a safe
change.

Yes, host_file is set to cfi->cfi_container but then we call
venus_access_intent, which is an upcall to userspace which involves a
context switch and may update the state of the container file.

I am going to have to look at this a bit more carefully because we do
use that cached host_file pointer to get i_size right after the write,
so maybe it is safe and as far as I know our userspace cache manager
only updates in place and never actually swaps the container file.

I haven't looked at this in a while, but I think f_pos synchronization
was assumed to run through the Coda inode, so the append used the
coda_inode->i_size to decide where the write would land, but I can also
see that with the locks being where they are and the access_intent upcalls
that may not be a valid assumption anymore.

You are correct that we cannot rely on the containers open time flags
because that container file was opened by the userspace cache manager
and is passed as an open fd to the kernel.  This handle is then shared
by all users of the Coda inode, so some Coda file handles could be
O_RDONLY, others may be O_RDWR, and some O_APPEND.

So I think the core of your patch is probably right, but it may need
some more careful thought, maybe the use of host_file is ok, maybe the
i_size needs to be copied from cfi->cfi_container instead of host_file.

Jan


> can therefore overwrite existing data instead of appending. For example,
> writing "Z" to a file containing "abc" produces "Zbc" rather than "abcZ".
> 
> Pass RWF_APPEND or RWF_NOAPPEND to vfs_iter_write() according to the
> incoming IOCB_APPEND state. This also honors changes made with F_SETFL
> and per-write append overrides instead of relying on the container's
> open-time flags.
> 
> Fixes: 164f4064ca81 ("keep iocb_flags() result cached in struct file")
> Cc: stable@vger.kernel.org # 6.9+
> Signed-off-by: Hungyu Lin <dennylin0707@gmail.com>
> ---
>  fs/coda/file.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/coda/file.c b/fs/coda/file.c
> index 6ca8bfcb58f2..ae03f0b30d84 100644
> --- a/fs/coda/file.c
> +++ b/fs/coda/file.c
> @@ -71,6 +71,7 @@ coda_file_write_iter(struct kiocb *iocb, struct iov_iter *to)
>  	struct file *host_file = cfi->cfi_container;
>  	loff_t ki_pos = iocb->ki_pos;
>  	size_t count = iov_iter_count(to);
> +	rwf_t flags = (iocb->ki_flags & IOCB_APPEND) ? RWF_APPEND : RWF_NOAPPEND;
>  	ssize_t ret;
>  
>  	ret = venus_access_intent(coda_inode->i_sb, coda_i2f(coda_inode),
> @@ -80,7 +81,7 @@ coda_file_write_iter(struct kiocb *iocb, struct iov_iter *to)
>  		goto finish_write;
>  
>  	inode_lock(coda_inode);
> -	ret = vfs_iter_write(cfi->cfi_container, to, &iocb->ki_pos, 0);
> +	ret = vfs_iter_write(host_file, to, &iocb->ki_pos, flags);
>  	coda_inode->i_size = file_inode(host_file)->i_size;
>  	coda_inode->i_blocks = (coda_inode->i_size + 511) >> 9;
>  	inode_set_mtime_to_ts(coda_inode, inode_set_ctime_current(coda_inode));
> 
> base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
> -- 
> 2.43.0
>