linux-kernel - Re: [PATCH 1/2] ceph/mds_client: transfer CEPH_CAP_PIN when updating r

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAO8a2ShM3yar0g83z=aC9VLuPz5bEpBAN8FxRqEqQfSnkgZGUw@mail.gmail.com>
Date: Wed, 3 Sep 2025 11:14:38 +0300
From: Alex Markuze <amarkuze@...hat.com>
To: Viacheslav Dubeyko <Slava.Dubeyko@....com>
Cc: "ceph-devel@...r.kernel.org" <ceph-devel@...r.kernel.org>, "idryomov@...il.com" <idryomov@...il.com>, 
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/2] ceph/mds_client: transfer CEPH_CAP_PIN when updating
 r_parent on mismatch

These patches apply to the testing branch. They update the r_parent race fix.

commit a69ac54928a45ad66b6ba84f9bd4be2fd0f9518e
Author: Alex Markuze <amarkuze@...hat.com>
Date:   Tue Aug 12 09:57:39 2025 +0000

    ceph: fix race condition where r_parent becomes stale before sending message

    When the parent directory's i_rwsem is not locked, req->r_parent may become
    stale due to concurrent operations (e.g. rename) between dentry lookup and
    message creation. Validate that r_parent matches the encoded parent inode
    and update to the correct inode if a mismatch is detected.

    Signed-off-by: Alex Markuze <amarkuze@...hat.com>
    Reviewed-by: Viacheslav Dubeyko <Slava.Dubeyko@....com>
    Signed-off-by: Ilya Dryomov <idryomov@...il.com>

commit 7128e41a490709c759fde32898eade197acd0978
Author: Alex Markuze <amarkuze@...hat.com>
Date:   Tue Aug 12 09:57:38 2025 +0000

    ceph: fix race condition validating r_parent before applying state

    Add validation to ensure the cached parent directory inode matches the
    directory info in MDS replies. This prevents client-side race conditions
    where concurrent operations (e.g. rename) cause r_parent to become stale
    between request initiation and reply processing, which could lead to
    applying state changes to incorrect directory inodes.

    Signed-off-by: Alex Markuze <amarkuze@...hat.com>
    Reviewed-by: Viacheslav Dubeyko <Slava.Dubeyko@....com>
    Signed-off-by: Ilya Dryomov <idryomov@...il.com>

On Tue, Sep 2, 2025 at 9:42 PM Viacheslav Dubeyko <Slava.Dubeyko@....com> wrote:
>
> On Mon, 2025-09-01 at 15:14 +0000, Alex Markuze wrote:
> > When the parent directory lock is not held, req->r_parent can become stale between dentry lookup and request encoding.
> > The client updates r_parent to the correct inode based on the encoded path, but previously did not adjust CEPH_CAP_PIN references.
> >
> > Release the pin from the old parent and acquire it for the new parent when switching r_parent, ensuring reference accounting stays balanced and avoiding leaks or underflows later in ceph_mdsc_release_request().
> >
>
> I cannot apply the patch on current state of the kernel. I assume that this
> patch is improvement of previous patch set. If so, then it will be better to
> send another version of previous patch set. Otherwise, it's hard to review and
> impossible to test it.
>
> Thanks,
> Slava.
>
> > Signed-off-by: Alex Markuze <amarkuze@...hat.com>
> > ---
> >  fs/ceph/mds_client.c | 11 +++++++++--
> >  1 file changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
> > index ce0c129f4651..4e5926f36e8d 100644
> > --- a/fs/ceph/mds_client.c
> > +++ b/fs/ceph/mds_client.c
> > @@ -3053,12 +3053,19 @@ static struct ceph_msg *create_request_message(struct ceph_mds_session *session,
> >        */
> >       if (!parent_locked && req->r_parent && path_info1.vino.ino &&
> >           ceph_ino(req->r_parent) != path_info1.vino.ino) {
> > +             struct inode *old_parent = req->r_parent;
> >               struct inode *correct_dir = ceph_get_inode(mdsc->fsc->sb, path_info1.vino, NULL);
> >               if (!IS_ERR(correct_dir)) {
> >                       WARN_ONCE(1, "ceph: r_parent mismatch (had %llx wanted %llx) - updating\n",
> > -                               ceph_ino(req->r_parent), path_info1.vino.ino);
> > -                     iput(req->r_parent);
> > +                               ceph_ino(old_parent), path_info1.vino.ino);
> > +                     /*
> > +                      * Transfer CEPH_CAP_PIN from the old parent to the new one.
> > +                      * The pin was taken earlier in ceph_mdsc_submit_request().
> > +                      */
> > +                     ceph_put_cap_refs(ceph_inode(old_parent), CEPH_CAP_PIN);
> > +                     iput(old_parent);
> >                       req->r_parent = correct_dir;
> > +                     ceph_get_cap_refs(ceph_inode(req->r_parent), CEPH_CAP_PIN);
> >               }
> >       }
> >