Re: [PATCH 5/5] ceph: narrow mdsc->mutex scope in replay_unsafe_requests

[Xiubo Li]

On Wed, 15 Jul 2026 at 03:07, Viacheslav Dubeyko <slava@xxxxxxxxxxx> wrote:
>
> On Mon, 2026-07-13 at 17:46 +0800, Xiubo Li via B4 Relay wrote:
> > From: Xiubo Li <xiubo.li@xxxxxxxxx>
[......]
> > @@ -4673,12 +4673,25 @@ static void replay_unsafe_requests(struct
> > ceph_mds_client *mdsc,
> >  {
> >       struct ceph_mds_request *req, *nreq;
> >       unsigned long idx;
> > +     LIST_HEAD(replay_list);
> >
> >       doutc(mdsc->fsc->client, "mds%d\n", session->s_mds);
> >
> > +     /* collect unsafe requests under the mutex */
> >       mutex_lock(&mdsc->mutex);
> > -     list_for_each_entry_safe(req, nreq, &session->s_unsafe,
> > r_unsafe_item)
> > +     list_for_each_entry_safe(req, nreq, &session->s_unsafe,
> > +                              r_unsafe_item) {
> > +             ceph_mdsc_get_request(req);
> > +             list_move(&req->r_unsafe_item, &replay_list);
> > +     }
> > +     mutex_unlock(&mdsc->mutex);
>
> This loop is done right.
>
> > +
> > +     /* replay unsafe requests — __send_request is lockless */
> > +     list_for_each_entry_safe(req, nreq, &replay_list,
> > r_unsafe_item) {
> >               __send_request(session, req, true);
> > +             list_del_init(&req->r_unsafe_item);
> > +             ceph_mdsc_put_request(req);
> > +     }
>
> However, this one has issue, if I am right. No ceph_mdsc_get_request()
> anywhere in it. It has always relied entirely on the caller holding
> mdsc->mutex for the duration of the loop.
>
> While this loop is inside __send_request() or
> ceph_mdsc_release_dir_caps_async() for one req, nothing stops another
> thread from acquiring mdsc->mutex and completing/timing-out/aborting
> that same request through the now-fully-unlocked
> __do_request()/wait_requests() path, dropping the last reference and
> freeing it — at which point the next loop iteration's xa_find() touches
> freed memory. Am I right here?
>

Good catch. The old-request loop in phase 3 iterates raw pointers from
xa_for_each() / rb_next() without holding a reference. If another
thread completes the request while __send_request() runs without
mdsc->mutex, the request could be freed, leading to UAF.

I will fix it.

Thanks
Xiubo LI