Re: [PATCH] ceph: revalidate ki_pos for O_APPEND writes after cap acquisition

Hi Slava,

On Sat, 18 Jul 2026 at 01:35, Viacheslav Dubeyko <[email protected]> wrote:
>
> On Fri, 2026-07-17 at 17:33 +0800, Xiubo Li via B4 Relay wrote:
> > From: Xiubo Li <[email protected]>
> >
> > For O_APPEND writes, ki_pos is set to the current EOF via
> > generic_write_checks() after fetching i_size from the MDS.  However,
> > ceph_get_caps() may need to wait for Fwx exclusive caps if the write
> > extends the file (endoff > i_max_size).  While waiting for Fwx, the
> > previous Fwx holder (another client) may have already extended the
> > file.  When the MDS grants us Fwx, the cap grant message updates the
> > local i_size, but ki_pos remains at the old EOF, causing the append
> > write to land at a stale offset and overwrite data from the other
> > client.
> >
> > Fix by re-reading i_size_read(inode) after ceph_get_caps() returns.
> > At this point we hold Fwx exclusive caps, no other client can modify
> > the file, and i_size reflects the true EOF from the MDS cap grant.
> > No extra MDS round-trip is needed.  Only adjust ki_pos when the EOF
> > has actually changed.
> >
> > Link: https://tracker.ceph.com/issues/7333
> > Signed-off-by: Xiubo Li <[email protected]>
> > ---
> > For O_APPEND writes, ki_pos is set to EOF via generic_write_checks()
> > after fetching i_size from the MDS via ceph_do_getattr().  However,
> > ceph_get_caps() may wait for Fwx exclusive caps (because endoff >
> > i_max_size).  During that wait, the previous Fwx holder may have
> > already extended the file.  When the MDS grants us Fwx, the local
> > i_size is updated via the cap grant message, but ki_pos is still
> > the old EOF, resulting in the append write overwriting data.
> >
> > Fix by revalidating ki_pos against i_size_read(inode) after
> > ceph_get_caps() returns.  We now hold Fwx — no other client can
> > change the file, so i_size is stable and correct.
> >
> > Fixes: 8e4473bb50a1 ("ceph: do not execute direct write in parallel
> > if O_APPEND is specified")
> > Link: https://tracker.ceph.com/issues/7333
>
> The patch has two --- separators. The Fixes and Link will be finally
> dropped.
>

I used the B4 and just added this to the cover letter. Let me fix it.

> > ---
> >  fs/ceph/file.c | 26 ++++++++++++++++++++++++++
> >  1 file changed, 26 insertions(+)
> >
> > diff --git a/fs/ceph/file.c b/fs/ceph/file.c
> > index 9d89d7fc1095..b35c9aee48dd 100644
> > --- a/fs/ceph/file.c
> > +++ b/fs/ceph/file.c
> > @@ -2491,6 +2491,32 @@ static ssize_t ceph_write_iter(struct kiocb
> > *iocb, struct iov_iter *from)
> >       if (err < 0)
> >               goto out;
>
> ceph_get_caps() validated caps against the old endoff = pos + count.
> After adjusting pos forward, the real write range is [cur_eof, cur_eof
> + count), which isn't re-validated against i_max_size. Could it be a
> potential issue?
>

Yeah, Good catch. We should fix this anyway. Let me fix it.

Thanks
Xiubo

> Thanks,
> Slava.
[......]