* [PATCH v2] ceph: fix possible NULL pointer dereference for req->r_session
@ 2022-04-18 1:44 Xiubo Li
2022-04-18 9:54 ` Jeff Layton
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Xiubo Li @ 2022-04-18 1:44 UTC (permalink / raw)
To: jlayton; +Cc: idryomov, vshankar, ceph-devel, Xiubo Li
The request will be inserted into the ci->i_unsafe_dirops before
assigning the req->r_session, so it's possible that we will hit
NULL pointer dereference bug here.
URL: https://tracker.ceph.com/issues/55327
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 69af17df59be..c70fd747c914 100644
--- a/fs/ceph/caps.c
+++ b/fs/ceph/caps.c
@@ -2333,6 +2333,8 @@ static int unsafe_request_wait(struct inode *inode)
list_for_each_entry(req, &ci->i_unsafe_dirops,
r_unsafe_dir_item) {
s = req->r_session;
+ if (!s)
+ continue;
if (unlikely(s->s_mds >= max_sessions)) {
spin_unlock(&ci->i_unsafe_lock);
for (i = 0; i < max_sessions; i++) {
@@ -2353,6 +2355,8 @@ static int unsafe_request_wait(struct inode *inode)
list_for_each_entry(req, &ci->i_unsafe_iops,
r_unsafe_target_item) {
s = req->r_session;
+ if (!s)
+ continue;
if (unlikely(s->s_mds >= max_sessions)) {
spin_unlock(&ci->i_unsafe_lock);
for (i = 0; i < max_sessions; i++) {
--
2.36.0.rc1
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v2] ceph: fix possible NULL pointer dereference for req->r_session
2022-04-18 1:44 [PATCH v2] ceph: fix possible NULL pointer dereference for req->r_session Xiubo Li
@ 2022-04-18 9:54 ` Jeff Layton
2022-04-18 10:41 ` Aaron Tomlin
2022-04-18 10:43 ` Aaron Tomlin
2 siblings, 0 replies; 9+ messages in thread
From: Jeff Layton @ 2022-04-18 9:54 UTC (permalink / raw)
To: Xiubo Li; +Cc: idryomov, vshankar, ceph-devel
On Mon, 2022-04-18 at 09:44 +0800, Xiubo Li wrote:
> The request will be inserted into the ci->i_unsafe_dirops before
> assigning the req->r_session, so it's possible that we will hit
> NULL pointer dereference bug here.
>
> URL: https://tracker.ceph.com/issues/55327
> 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 69af17df59be..c70fd747c914 100644
> --- a/fs/ceph/caps.c
> +++ b/fs/ceph/caps.c
> @@ -2333,6 +2333,8 @@ static int unsafe_request_wait(struct inode *inode)
> list_for_each_entry(req, &ci->i_unsafe_dirops,
> r_unsafe_dir_item) {
> s = req->r_session;
> + if (!s)
> + continue;
> if (unlikely(s->s_mds >= max_sessions)) {
> spin_unlock(&ci->i_unsafe_lock);
> for (i = 0; i < max_sessions; i++) {
> @@ -2353,6 +2355,8 @@ static int unsafe_request_wait(struct inode *inode)
> list_for_each_entry(req, &ci->i_unsafe_iops,
> r_unsafe_target_item) {
> s = req->r_session;
> + if (!s)
> + continue;
> if (unlikely(s->s_mds >= max_sessions)) {
> spin_unlock(&ci->i_unsafe_lock);
> for (i = 0; i < max_sessions; i++) {
Reviewed-by: Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v2] ceph: fix possible NULL pointer dereference for req->r_session
2022-04-18 1:44 [PATCH v2] ceph: fix possible NULL pointer dereference for req->r_session Xiubo Li
2022-04-18 9:54 ` Jeff Layton
@ 2022-04-18 10:41 ` Aaron Tomlin
2022-04-18 10:43 ` Aaron Tomlin
2 siblings, 0 replies; 9+ messages in thread
From: Aaron Tomlin @ 2022-04-18 10:41 UTC (permalink / raw)
To: Xiubo Li; +Cc: jlayton, idryomov, vshankar, ceph-devel
On Mon 2022-04-18 09:44 +0800, Xiubo Li wrote:
> The request will be inserted into the ci->i_unsafe_dirops before
> assigning the req->r_session, so it's possible that we will hit
> NULL pointer dereference bug here.
>
> URL: https://tracker.ceph.com/issues/55327
> 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 69af17df59be..c70fd747c914 100644
> --- a/fs/ceph/caps.c
> +++ b/fs/ceph/caps.c
> @@ -2333,6 +2333,8 @@ static int unsafe_request_wait(struct inode *inode)
> list_for_each_entry(req, &ci->i_unsafe_dirops,
> r_unsafe_dir_item) {
> s = req->r_session;
> + if (!s)
> + continue;
> if (unlikely(s->s_mds >= max_sessions)) {
> spin_unlock(&ci->i_unsafe_lock);
> for (i = 0; i < max_sessions; i++) {
> @@ -2353,6 +2355,8 @@ static int unsafe_request_wait(struct inode *inode)
> list_for_each_entry(req, &ci->i_unsafe_iops,
> r_unsafe_target_item) {
> s = req->r_session;
> + if (!s)
> + continue;
> if (unlikely(s->s_mds >= max_sessions)) {
> spin_unlock(&ci->i_unsafe_lock);
> for (i = 0; i < max_sessions; i++) {
> --
> 2.36.0.rc1
Thanks Xiubo!
Reviewed-by: Aaron Tomlin <atomlin@redhat.com>
--
Aaron Tomlin
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v2] ceph: fix possible NULL pointer dereference for req->r_session
2022-04-18 1:44 [PATCH v2] ceph: fix possible NULL pointer dereference for req->r_session Xiubo Li
2022-04-18 9:54 ` Jeff Layton
2022-04-18 10:41 ` Aaron Tomlin
@ 2022-04-18 10:43 ` Aaron Tomlin
2022-04-18 10:52 ` Xiubo Li
2 siblings, 1 reply; 9+ messages in thread
From: Aaron Tomlin @ 2022-04-18 10:43 UTC (permalink / raw)
To: Xiubo Li; +Cc: jlayton, idryomov, vshankar, ceph-devel
On Mon 2022-04-18 09:44 +0800, Xiubo Li wrote:
> The request will be inserted into the ci->i_unsafe_dirops before
> assigning the req->r_session, so it's possible that we will hit
> NULL pointer dereference bug here.
>
> URL: https://tracker.ceph.com/issues/55327
> 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 69af17df59be..c70fd747c914 100644
> --- a/fs/ceph/caps.c
> +++ b/fs/ceph/caps.c
> @@ -2333,6 +2333,8 @@ static int unsafe_request_wait(struct inode *inode)
> list_for_each_entry(req, &ci->i_unsafe_dirops,
> r_unsafe_dir_item) {
> s = req->r_session;
> + if (!s)
> + continue;
> if (unlikely(s->s_mds >= max_sessions)) {
> spin_unlock(&ci->i_unsafe_lock);
> for (i = 0; i < max_sessions; i++) {
> @@ -2353,6 +2355,8 @@ static int unsafe_request_wait(struct inode *inode)
> list_for_each_entry(req, &ci->i_unsafe_iops,
> r_unsafe_target_item) {
> s = req->r_session;
> + if (!s)
> + continue;
> if (unlikely(s->s_mds >= max_sessions)) {
> spin_unlock(&ci->i_unsafe_lock);
> for (i = 0; i < max_sessions; i++) {
> --
> 2.36.0.rc1
Tested-by: Aaron Tomlin <atomlin@redhat.com>
--
Aaron Tomlin
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v2] ceph: fix possible NULL pointer dereference for req->r_session
2022-04-18 10:43 ` Aaron Tomlin
@ 2022-04-18 10:52 ` Xiubo Li
2022-04-18 14:45 ` Aaron Tomlin
0 siblings, 1 reply; 9+ messages in thread
From: Xiubo Li @ 2022-04-18 10:52 UTC (permalink / raw)
To: Aaron Tomlin; +Cc: jlayton, idryomov, vshankar, ceph-devel
On 4/18/22 6:43 PM, Aaron Tomlin wrote:
> On Mon 2022-04-18 09:44 +0800, Xiubo Li wrote:
>> The request will be inserted into the ci->i_unsafe_dirops before
>> assigning the req->r_session, so it's possible that we will hit
>> NULL pointer dereference bug here.
>>
>> URL: https://tracker.ceph.com/issues/55327
>> 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 69af17df59be..c70fd747c914 100644
>> --- a/fs/ceph/caps.c
>> +++ b/fs/ceph/caps.c
>> @@ -2333,6 +2333,8 @@ static int unsafe_request_wait(struct inode *inode)
>> list_for_each_entry(req, &ci->i_unsafe_dirops,
>> r_unsafe_dir_item) {
>> s = req->r_session;
>> + if (!s)
>> + continue;
>> if (unlikely(s->s_mds >= max_sessions)) {
>> spin_unlock(&ci->i_unsafe_lock);
>> for (i = 0; i < max_sessions; i++) {
>> @@ -2353,6 +2355,8 @@ static int unsafe_request_wait(struct inode *inode)
>> list_for_each_entry(req, &ci->i_unsafe_iops,
>> r_unsafe_target_item) {
>> s = req->r_session;
>> + if (!s)
>> + continue;
>> if (unlikely(s->s_mds >= max_sessions)) {
>> spin_unlock(&ci->i_unsafe_lock);
>> for (i = 0; i < max_sessions; i++) {
>> --
>> 2.36.0.rc1
> Tested-by: Aaron Tomlin <atomlin@redhat.com>
>
Hi Aaron,
Thanks very much for you testing.
BTW, did you test this by using Livepatch or something else ?
-- Xiubo
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v2] ceph: fix possible NULL pointer dereference for req->r_session
2022-04-18 10:52 ` Xiubo Li
@ 2022-04-18 14:45 ` Aaron Tomlin
2022-04-19 1:01 ` Xiubo Li
0 siblings, 1 reply; 9+ messages in thread
From: Aaron Tomlin @ 2022-04-18 14:45 UTC (permalink / raw)
To: Xiubo Li; +Cc: jlayton, idryomov, vshankar, ceph-devel
On Mon 2022-04-18 18:52 +0800, Xiubo Li wrote:
> Hi Aaron,
Hi Xiubo,
> Thanks very much for you testing.
No problem!
> BTW, did you test this by using Livepatch or something else ?
I mostly followed your suggestion here [1] by modifying/or patching the
kernel to increase the race window so that unsafe_request_wait() may more
reliably see a newly registered request with an unprepared session pointer
i.e. 'req->r_session == NULL'.
Indeed, with this patch we simply skip such a request while traversing the
Ceph inode's unsafe directory list in the context of unsafe_request_wait().
[1]: https://tracker.ceph.com/issues/55329
Kind regards,
--
Aaron Tomlin
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] ceph: fix possible NULL pointer dereference for req->r_session
2022-04-18 14:45 ` Aaron Tomlin
@ 2022-04-19 1:01 ` Xiubo Li
2022-04-25 9:03 ` Ilya Dryomov
0 siblings, 1 reply; 9+ messages in thread
From: Xiubo Li @ 2022-04-19 1:01 UTC (permalink / raw)
To: Aaron Tomlin; +Cc: jlayton, idryomov, vshankar, ceph-devel
On 4/18/22 10:45 PM, Aaron Tomlin wrote:
> On Mon 2022-04-18 18:52 +0800, Xiubo Li wrote:
>> Hi Aaron,
> Hi Xiubo,
>
>> Thanks very much for you testing.
> No problem!
>
>> BTW, did you test this by using Livepatch or something else ?
> I mostly followed your suggestion here [1] by modifying/or patching the
> kernel to increase the race window so that unsafe_request_wait() may more
> reliably see a newly registered request with an unprepared session pointer
> i.e. 'req->r_session == NULL'.
>
> Indeed, with this patch we simply skip such a request while traversing the
> Ceph inode's unsafe directory list in the context of unsafe_request_wait().
Okay, cool.
Thanks again for your help Aaron :-)
-- Xiubo
> [1]: https://tracker.ceph.com/issues/55329
>
> Kind regards,
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] ceph: fix possible NULL pointer dereference for req->r_session
2022-04-19 1:01 ` Xiubo Li
@ 2022-04-25 9:03 ` Ilya Dryomov
2022-04-25 9:23 ` Xiubo Li
0 siblings, 1 reply; 9+ messages in thread
From: Ilya Dryomov @ 2022-04-25 9:03 UTC (permalink / raw)
To: Xiubo Li; +Cc: Aaron Tomlin, Jeff Layton, Venky Shankar, Ceph Development
On Tue, Apr 19, 2022 at 3:01 AM Xiubo Li <xiubli@redhat.com> wrote:
>
>
> On 4/18/22 10:45 PM, Aaron Tomlin wrote:
> > On Mon 2022-04-18 18:52 +0800, Xiubo Li wrote:
> >> Hi Aaron,
> > Hi Xiubo,
> >
> >> Thanks very much for you testing.
> > No problem!
> >
> >> BTW, did you test this by using Livepatch or something else ?
> > I mostly followed your suggestion here [1] by modifying/or patching the
> > kernel to increase the race window so that unsafe_request_wait() may more
> > reliably see a newly registered request with an unprepared session pointer
> > i.e. 'req->r_session == NULL'.
> >
> > Indeed, with this patch we simply skip such a request while traversing the
> > Ceph inode's unsafe directory list in the context of unsafe_request_wait().
>
> Okay, cool.
>
> Thanks again for your help Aaron :-)
>
> -- Xiubo
>
>
> > [1]: https://tracker.ceph.com/issues/55329
> >
> > Kind regards,
> >
>
I went ahead and marked this for stable (it's already queued for -rc5).
Thanks,
Ilya
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v2] ceph: fix possible NULL pointer dereference for req->r_session
2022-04-25 9:03 ` Ilya Dryomov
@ 2022-04-25 9:23 ` Xiubo Li
0 siblings, 0 replies; 9+ messages in thread
From: Xiubo Li @ 2022-04-25 9:23 UTC (permalink / raw)
To: Ilya Dryomov; +Cc: Aaron Tomlin, Jeff Layton, Venky Shankar, Ceph Development
On 4/25/22 5:03 PM, Ilya Dryomov wrote:
> On Tue, Apr 19, 2022 at 3:01 AM Xiubo Li <xiubli@redhat.com> wrote:
>>
>> On 4/18/22 10:45 PM, Aaron Tomlin wrote:
>>> On Mon 2022-04-18 18:52 +0800, Xiubo Li wrote:
>>>> Hi Aaron,
>>> Hi Xiubo,
>>>
>>>> Thanks very much for you testing.
>>> No problem!
>>>
>>>> BTW, did you test this by using Livepatch or something else ?
>>> I mostly followed your suggestion here [1] by modifying/or patching the
>>> kernel to increase the race window so that unsafe_request_wait() may more
>>> reliably see a newly registered request with an unprepared session pointer
>>> i.e. 'req->r_session == NULL'.
>>>
>>> Indeed, with this patch we simply skip such a request while traversing the
>>> Ceph inode's unsafe directory list in the context of unsafe_request_wait().
>> Okay, cool.
>>
>> Thanks again for your help Aaron :-)
>>
>> -- Xiubo
>>
>>
>>> [1]: https://tracker.ceph.com/issues/55329
>>>
>>> Kind regards,
>>>
> I went ahead and marked this for stable (it's already queued for -rc5).
Sure, thanks Ilya.
-- Xiubo
> Thanks,
>
> Ilya
>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-04-25 9:23 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-18 1:44 [PATCH v2] ceph: fix possible NULL pointer dereference for req->r_session Xiubo Li
2022-04-18 9:54 ` Jeff Layton
2022-04-18 10:41 ` Aaron Tomlin
2022-04-18 10:43 ` Aaron Tomlin
2022-04-18 10:52 ` Xiubo Li
2022-04-18 14:45 ` Aaron Tomlin
2022-04-19 1:01 ` Xiubo Li
2022-04-25 9:03 ` Ilya Dryomov
2022-04-25 9:23 ` 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.