On Wed, 2020-07-01 at 19:25 +0800, Xiubo Li wrote: > On 2020/7/1 19:17, Jeff Layton wrote: > > On Wed, 2020-07-01 at 01:52 -0400, xiubli@xxxxxxxxxx wrote: > > > From: Xiubo Li <xiubli@xxxxxxxxxx> > > > > > > Make sure the delayed work stopped before releasing the resources. > > > > > > Because the cancel_delayed_work_sync() will only guarantee that the > > > work finishes executing if the work is already in the ->worklist. > > > That means after the cancel_delayed_work_sync() returns and in case > > > if the work will re-arm itself, it will leave the work requeued. And > > > if we release the resources before the delayed work to run again we > > > will hit the use-after-free bug. > > > > > > URL: https://tracker.ceph.com/issues/46293 > > > Signed-off-by: Xiubo Li <xiubli@xxxxxxxxxx> > > > --- > > > fs/ceph/mds_client.c | 14 +++++++++++++- > > > 1 file changed, 13 insertions(+), 1 deletion(-) > > > > > > diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c > > > index d5e523c..9a09d12 100644 > > > --- a/fs/ceph/mds_client.c > > > +++ b/fs/ceph/mds_client.c > > > @@ -4330,6 +4330,9 @@ static void delayed_work(struct work_struct *work) > > > > > > dout("mdsc delayed_work\n"); > > > > > > + if (mdsc->stopping) > > > + return; > > > + > > > mutex_lock(&mdsc->mutex); > > > renew_interval = mdsc->mdsmap->m_session_timeout >> 2; > > > renew_caps = time_after_eq(jiffies, HZ*renew_interval + > > > @@ -4689,7 +4692,16 @@ void ceph_mdsc_force_umount(struct ceph_mds_client *mdsc) > > > static void ceph_mdsc_stop(struct ceph_mds_client *mdsc) > > > { > > > dout("stop\n"); > > > - cancel_delayed_work_sync(&mdsc->delayed_work); /* cancel timer */ > > > + /* > > > + * Make sure the delayed work stopped before releasing > > > + * the resources. > > > + * > > > + * Because the cancel_delayed_work_sync() will only > > > + * guarantee that the work finishes executing. But the > > > + * delayed work will re-arm itself again after that. > > > + */ > > > + flush_delayed_work(&mdsc->delayed_work); > > > + > > > if (mdsc->mdsmap) > > > ceph_mdsmap_destroy(mdsc->mdsmap); > > > kfree(mdsc->sessions); > > This patch looks fine, but the subject says [PATCH 1/3]. Were there > > others in this series that didn't make it to the list for some reason? > > Sorry for confusing. > > Just generated this patch with the metrics series and forget to fix it > before sending out. > > No worries. Just making sure before I merged it. Merged patch into testing branch. Thanks! -- Jeff Layton <jlayton@xxxxxxxxxx>
