diff mbox series

ceph: do not touch cap when trimming the caps

Message ID 20230414024123.263120-1-xiubli@redhat.com
State New
Headers show
Series ceph: do not touch cap when trimming the caps | expand

Commit Message

Xiubo Li April 14, 2023, 2:41 a.m. UTC
From: Xiubo Li <xiubli@redhat.com>

When trimming the caps it maybe queued to release in the next loop,
and just after the 'session->s_cap_lock' lock is released the
'session->s_cap_iterator' will be set to NULL and the cap also has
been removed from 'session->s_caps' list, then the '__touch_cap()'
could continue and add the cap back to the 'session->s_caps' list.

That means this cap could be iterated twice to call 'trim_caps_cb()'
and the second time will trigger use-after-free bug.

Cc: stable@vger.kernel.org
URL: https://bugzilla.redhat.com/show_bug.cgi?id=2186264
Signed-off-by: Xiubo Li <xiubli@redhat.com>
---
 fs/ceph/caps.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Xiubo Li April 14, 2023, 4:28 a.m. UTC | #1
On 4/14/23 10:41, xiubli@redhat.com wrote:
> From: Xiubo Li <xiubli@redhat.com>
>
> When trimming the caps it maybe queued to release in the next loop,
> and just after the 'session->s_cap_lock' lock is released the
> 'session->s_cap_iterator' will be set to NULL and the cap also has
> been removed from 'session->s_caps' list, then the '__touch_cap()'
> could continue and add the cap back to the 'session->s_caps' list.
>
> That means this cap could be iterated twice to call 'trim_caps_cb()'
> and the second time will trigger use-after-free bug.
>
> Cc: stable@vger.kernel.org
> URL: https://bugzilla.redhat.com/show_bug.cgi?id=2186264
> Signed-off-by: Xiubo Li <xiubli@redhat.com>
> ---
>   fs/ceph/caps.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
> index cf29e395af23..186c9818ab0d 100644
> --- a/fs/ceph/caps.c
> +++ b/fs/ceph/caps.c
> @@ -846,7 +846,7 @@ static void __touch_cap(struct ceph_cap *cap)
>   	struct ceph_mds_session *s = cap->session;
>   
>   	spin_lock(&s->s_cap_lock);
> -	if (!s->s_cap_iterator) {
> +	if (!s->s_cap_iterator && !list_empty(&cap->session_caps) && !cap->queue_release) {

Comment it myself.

The s_cap_iterator will always be true during trimming the caps. So this 
check here is incorrect.

>   		dout("__touch_cap %p cap %p mds%d\n", &cap->ci->netfs.inode, cap,
>   		     s->s_mds);
>   		list_move_tail(&cap->session_caps, &s->s_caps);
diff mbox series

Patch

diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
index cf29e395af23..186c9818ab0d 100644
--- a/fs/ceph/caps.c
+++ b/fs/ceph/caps.c
@@ -846,7 +846,7 @@  static void __touch_cap(struct ceph_cap *cap)
 	struct ceph_mds_session *s = cap->session;
 
 	spin_lock(&s->s_cap_lock);
-	if (!s->s_cap_iterator) {
+	if (!s->s_cap_iterator && !list_empty(&cap->session_caps) && !cap->queue_release) {
 		dout("__touch_cap %p cap %p mds%d\n", &cap->ci->netfs.inode, cap,
 		     s->s_mds);
 		list_move_tail(&cap->session_caps, &s->s_caps);