| Message ID | 20221027091155.334178-1-xiubli@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ceph: fix NULL pointer dereference for req->r_session | expand |
On Thu, Oct 27, 2022 at 11:12 AM <xiubli@redhat.com> wrote: > > From: Xiubo Li <xiubli@redhat.com> > > The request's r_session maybe changed when it was forwarded or > resent. > > Cc: stable@vger.kernel.org > Signed-off-by: Xiubo Li <xiubli@redhat.com> > --- > fs/ceph/caps.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c > index 894adfb4a092..d34ac716d7fe 100644 > --- a/fs/ceph/caps.c > +++ b/fs/ceph/caps.c > @@ -2341,6 +2341,7 @@ static int flush_mdlog_and_wait_inode_unsafe_requests(struct inode *inode) > goto out; > } > > + mutex_lock(&mdsc->mutex); Hi Xiubo, A few lines above, there is the following comment: /* * The mdsc->max_sessions is unlikely to be changed * mostly, here we will retry it by reallocating the * sessions array memory to get rid of the mdsc->mutex * lock. */ Does retry label and gotos still make sense if mdsc->mutex is introduced? Would it make sense to move it up and get rid of retry code? Thanks, Ilya
On 07/11/2022 22:55, Ilya Dryomov wrote: > On Thu, Oct 27, 2022 at 11:12 AM <xiubli@redhat.com> wrote: >> From: Xiubo Li <xiubli@redhat.com> >> >> The request's r_session maybe changed when it was forwarded or >> resent. >> >> Cc: stable@vger.kernel.org >> Signed-off-by: Xiubo Li <xiubli@redhat.com> >> --- >> fs/ceph/caps.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c >> index 894adfb4a092..d34ac716d7fe 100644 >> --- a/fs/ceph/caps.c >> +++ b/fs/ceph/caps.c >> @@ -2341,6 +2341,7 @@ static int flush_mdlog_and_wait_inode_unsafe_requests(struct inode *inode) >> goto out; >> } >> >> + mutex_lock(&mdsc->mutex); > Hi Xiubo, > > A few lines above, there is the following comment: > > /* > * The mdsc->max_sessions is unlikely to be changed > * mostly, here we will retry it by reallocating the > * sessions array memory to get rid of the mdsc->mutex > * lock. > */ > > Does retry label and gotos still make sense if mdsc->mutex is > introduced? Would it make sense to move it up and get rid of > retry code? I'm okay to remove the label since we will introduce the mdsc->mutex. Thanks! - Xiubo > Thanks, > > Ilya >
diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c index 894adfb4a092..d34ac716d7fe 100644 --- a/fs/ceph/caps.c +++ b/fs/ceph/caps.c @@ -2341,6 +2341,7 @@ static int flush_mdlog_and_wait_inode_unsafe_requests(struct inode *inode) goto out; } + mutex_lock(&mdsc->mutex); spin_lock(&ci->i_unsafe_lock); if (req1) { list_for_each_entry(req, &ci->i_unsafe_dirops, @@ -2350,6 +2351,7 @@ static int flush_mdlog_and_wait_inode_unsafe_requests(struct inode *inode) continue; if (unlikely(s->s_mds >= max_sessions)) { spin_unlock(&ci->i_unsafe_lock); + mutex_unlock(&mdsc->mutex); for (i = 0; i < max_sessions; i++) { s = sessions[i]; if (s) @@ -2372,6 +2374,7 @@ static int flush_mdlog_and_wait_inode_unsafe_requests(struct inode *inode) continue; if (unlikely(s->s_mds >= max_sessions)) { spin_unlock(&ci->i_unsafe_lock); + mutex_unlock(&mdsc->mutex); for (i = 0; i < max_sessions; i++) { s = sessions[i]; if (s) @@ -2387,6 +2390,7 @@ static int flush_mdlog_and_wait_inode_unsafe_requests(struct inode *inode) } } spin_unlock(&ci->i_unsafe_lock); + mutex_unlock(&mdsc->mutex); /* the auth MDS */ spin_lock(&ci->i_ceph_lock);