All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ceph: init the i_list/g_list for cap flush
@ 2021-08-25  5:22 xiubli
  2021-08-25 10:50 ` Jeff Layton
  0 siblings, 1 reply; 4+ messages in thread
From: xiubli @ 2021-08-25  5:22 UTC (permalink / raw)
  To: jlayton; +Cc: idryomov, ukernel, pdonnell, ceph-devel, Xiubo Li, stable

From: Xiubo Li <xiubli@redhat.com>

Always init the i_list/g_list in the begining to make sure it won't
crash the kernel if someone want to delete the cap_flush from the
lists.

Cc: stable@vger.kernel.org
URL: https://tracker.ceph.com/issues/52401
Signed-off-by: Xiubo Li <xiubli@redhat.com>
---
 fs/ceph/caps.c | 2 +-
 fs/ceph/snap.c | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
index 4f0dbc640b0b..60f60260cf42 100644
--- a/fs/ceph/caps.c
+++ b/fs/ceph/caps.c
@@ -3666,7 +3666,7 @@ static void handle_cap_flush_ack(struct inode *inode, u64 flush_tid,
 	while (!list_empty(&to_remove)) {
 		cf = list_first_entry(&to_remove,
 				      struct ceph_cap_flush, i_list);
-		list_del(&cf->i_list);
+		list_del_init(&cf->i_list);
 		if (!cf->is_capsnap)
 			ceph_free_cap_flush(cf);
 	}
diff --git a/fs/ceph/snap.c b/fs/ceph/snap.c
index 62fab59bbf96..b41e6724c591 100644
--- a/fs/ceph/snap.c
+++ b/fs/ceph/snap.c
@@ -488,6 +488,8 @@ static void ceph_queue_cap_snap(struct ceph_inode_info *ci)
 		return;
 	}
 	capsnap->cap_flush.is_capsnap = true;
+	INIT_LIST_HEAD(&capsnap->cap_flush.i_list);
+	INIT_LIST_HEAD(&capsnap->cap_flush.g_list);
 
 	spin_lock(&ci->i_ceph_lock);
 	used = __ceph_caps_used(ci);
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] ceph: init the i_list/g_list for cap flush
  2021-08-25  5:22 [PATCH] ceph: init the i_list/g_list for cap flush xiubli
@ 2021-08-25 10:50 ` Jeff Layton
  2021-08-25 14:31   ` Ilya Dryomov
  0 siblings, 1 reply; 4+ messages in thread
From: Jeff Layton @ 2021-08-25 10:50 UTC (permalink / raw)
  To: xiubli; +Cc: idryomov, ukernel, pdonnell, ceph-devel, stable

On Wed, 2021-08-25 at 13:22 +0800, xiubli@redhat.com wrote:
> From: Xiubo Li <xiubli@redhat.com>
> 
> Always init the i_list/g_list in the begining to make sure it won't
> crash the kernel if someone want to delete the cap_flush from the
> lists.
> 
> Cc: stable@vger.kernel.org
> URL: https://tracker.ceph.com/issues/52401
> Signed-off-by: Xiubo Li <xiubli@redhat.com>
> ---
>  fs/ceph/caps.c | 2 +-
>  fs/ceph/snap.c | 2 ++
>  2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
> index 4f0dbc640b0b..60f60260cf42 100644
> --- a/fs/ceph/caps.c
> +++ b/fs/ceph/caps.c
> @@ -3666,7 +3666,7 @@ static void handle_cap_flush_ack(struct inode *inode, u64 flush_tid,
>  	while (!list_empty(&to_remove)) {
>  		cf = list_first_entry(&to_remove,
>  				      struct ceph_cap_flush, i_list);
> -		list_del(&cf->i_list);
> +		list_del_init(&cf->i_list);
>  		if (!cf->is_capsnap)
>  			ceph_free_cap_flush(cf);
>  	}
> diff --git a/fs/ceph/snap.c b/fs/ceph/snap.c
> index 62fab59bbf96..b41e6724c591 100644
> --- a/fs/ceph/snap.c
> +++ b/fs/ceph/snap.c
> @@ -488,6 +488,8 @@ static void ceph_queue_cap_snap(struct ceph_inode_info *ci)
>  		return;
>  	}
>  	capsnap->cap_flush.is_capsnap = true;
> +	INIT_LIST_HEAD(&capsnap->cap_flush.i_list);
> +	INIT_LIST_HEAD(&capsnap->cap_flush.g_list);
>  
>  	spin_lock(&ci->i_ceph_lock);
>  	used = __ceph_caps_used(ci);

I'm not certain the second hunk is strictly needed. These either end up
on the list or they just get freed. That said, they shouldn't hurt
anything and it is more consistent. Merged into testing.

Ilya, since this is marked for stable, this probably ought to go to
Linus in the last v5.14 pile.

Thanks,
-- 
Jeff Layton <jlayton@kernel.org>


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] ceph: init the i_list/g_list for cap flush
  2021-08-25 10:50 ` Jeff Layton
@ 2021-08-25 14:31   ` Ilya Dryomov
  2021-08-26  0:34     ` Xiubo Li
  0 siblings, 1 reply; 4+ messages in thread
From: Ilya Dryomov @ 2021-08-25 14:31 UTC (permalink / raw)
  To: Jeff Layton
  Cc: Xiubo Li, Yan, Zheng, Patrick Donnelly, Ceph Development, stable

On Wed, Aug 25, 2021 at 12:50 PM Jeff Layton <jlayton@kernel.org> wrote:
>
> On Wed, 2021-08-25 at 13:22 +0800, xiubli@redhat.com wrote:
> > From: Xiubo Li <xiubli@redhat.com>
> >
> > Always init the i_list/g_list in the begining to make sure it won't
> > crash the kernel if someone want to delete the cap_flush from the
> > lists.
> >
> > Cc: stable@vger.kernel.org
> > URL: https://tracker.ceph.com/issues/52401
> > Signed-off-by: Xiubo Li <xiubli@redhat.com>
> > ---
> >  fs/ceph/caps.c | 2 +-
> >  fs/ceph/snap.c | 2 ++
> >  2 files changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
> > index 4f0dbc640b0b..60f60260cf42 100644
> > --- a/fs/ceph/caps.c
> > +++ b/fs/ceph/caps.c
> > @@ -3666,7 +3666,7 @@ static void handle_cap_flush_ack(struct inode *inode, u64 flush_tid,
> >       while (!list_empty(&to_remove)) {
> >               cf = list_first_entry(&to_remove,
> >                                     struct ceph_cap_flush, i_list);
> > -             list_del(&cf->i_list);
> > +             list_del_init(&cf->i_list);
> >               if (!cf->is_capsnap)
> >                       ceph_free_cap_flush(cf);
> >       }
> > diff --git a/fs/ceph/snap.c b/fs/ceph/snap.c
> > index 62fab59bbf96..b41e6724c591 100644
> > --- a/fs/ceph/snap.c
> > +++ b/fs/ceph/snap.c
> > @@ -488,6 +488,8 @@ static void ceph_queue_cap_snap(struct ceph_inode_info *ci)
> >               return;
> >       }
> >       capsnap->cap_flush.is_capsnap = true;
> > +     INIT_LIST_HEAD(&capsnap->cap_flush.i_list);
> > +     INIT_LIST_HEAD(&capsnap->cap_flush.g_list);
> >
> >       spin_lock(&ci->i_ceph_lock);
> >       used = __ceph_caps_used(ci);
>
> I'm not certain the second hunk is strictly needed. These either end up
> on the list or they just get freed. That said, they shouldn't hurt
> anything and it is more consistent. Merged into testing.
>
> Ilya, since this is marked for stable, this probably ought to go to
> Linus in the last v5.14 pile.

I'm inclined to fold this into "ceph: correctly handle releasing an
embedded cap flush" which is already queued for 5.14.

Thanks,

                Ilya

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] ceph: init the i_list/g_list for cap flush
  2021-08-25 14:31   ` Ilya Dryomov
@ 2021-08-26  0:34     ` Xiubo Li
  0 siblings, 0 replies; 4+ messages in thread
From: Xiubo Li @ 2021-08-26  0:34 UTC (permalink / raw)
  To: Ilya Dryomov, Jeff Layton
  Cc: Yan, Zheng, Patrick Donnelly, Ceph Development, stable


On 8/25/21 10:31 PM, Ilya Dryomov wrote:
> On Wed, Aug 25, 2021 at 12:50 PM Jeff Layton <jlayton@kernel.org> wrote:
>> On Wed, 2021-08-25 at 13:22 +0800, xiubli@redhat.com wrote:
>>> From: Xiubo Li <xiubli@redhat.com>
>>>
>>> Always init the i_list/g_list in the begining to make sure it won't
>>> crash the kernel if someone want to delete the cap_flush from the
>>> lists.
>>>
>>> Cc: stable@vger.kernel.org
>>> URL: https://tracker.ceph.com/issues/52401
>>> Signed-off-by: Xiubo Li <xiubli@redhat.com>
>>> ---
>>>   fs/ceph/caps.c | 2 +-
>>>   fs/ceph/snap.c | 2 ++
>>>   2 files changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
>>> index 4f0dbc640b0b..60f60260cf42 100644
>>> --- a/fs/ceph/caps.c
>>> +++ b/fs/ceph/caps.c
>>> @@ -3666,7 +3666,7 @@ static void handle_cap_flush_ack(struct inode *inode, u64 flush_tid,
>>>        while (!list_empty(&to_remove)) {
>>>                cf = list_first_entry(&to_remove,
>>>                                      struct ceph_cap_flush, i_list);
>>> -             list_del(&cf->i_list);
>>> +             list_del_init(&cf->i_list);
>>>                if (!cf->is_capsnap)
>>>                        ceph_free_cap_flush(cf);
>>>        }
>>> diff --git a/fs/ceph/snap.c b/fs/ceph/snap.c
>>> index 62fab59bbf96..b41e6724c591 100644
>>> --- a/fs/ceph/snap.c
>>> +++ b/fs/ceph/snap.c
>>> @@ -488,6 +488,8 @@ static void ceph_queue_cap_snap(struct ceph_inode_info *ci)
>>>                return;
>>>        }
>>>        capsnap->cap_flush.is_capsnap = true;
>>> +     INIT_LIST_HEAD(&capsnap->cap_flush.i_list);
>>> +     INIT_LIST_HEAD(&capsnap->cap_flush.g_list);
>>>
>>>        spin_lock(&ci->i_ceph_lock);
>>>        used = __ceph_caps_used(ci);
>> I'm not certain the second hunk is strictly needed. These either end up
>> on the list or they just get freed. That said, they shouldn't hurt
>> anything and it is more consistent. Merged into testing.
>>
>> Ilya, since this is marked for stable, this probably ought to go to
>> Linus in the last v5.14 pile.
> I'm inclined to fold this into "ceph: correctly handle releasing an
> embedded cap flush" which is already queued for 5.14.

Yeah, that's will be better.


>
> Thanks,
>
>                  Ilya
>


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-08-26  0:34 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-25  5:22 [PATCH] ceph: init the i_list/g_list for cap flush xiubli
2021-08-25 10:50 ` Jeff Layton
2021-08-25 14:31   ` Ilya Dryomov
2021-08-26  0:34     ` Xiubo Li

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.