[PATCH] ceph: fix NULL pointer dereference for req->r_session

[PATCH] ceph: fix NULL pointer dereference for req->r_session

[xiubli]

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);
 		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);
-- 
2.31.1

Re: [PATCH] ceph: fix NULL pointer dereference for req->r_session

[Ilya Dryomov]

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

Re: [PATCH] ceph: fix NULL pointer dereference for req->r_session

[Xiubo Li]

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
>