Re: [PATCH v1] trace: Fix race in trace_open and buffer resize call
On Sun, Jan 24, 2021 at 03:27:25PM +0530, Gaurav Kohli wrote: > > > On 1/24/2021 8:51 AM, Steven Rostedt wrote: > > On Sat, 23 Jan 2021 22:03:27 +0530 > > Gaurav Kohli wrote: > > > > > > > Sure I will do, I have never posted on backport branches. Let me check > > > and post it. > > > > > > > Basically you take your original patch that was in mainline (as the > > subject and commit message), and make it work as if you were doing the > > same exact fix for the stable release. > > > > Send it to me (and Cc everyone else), and I'll give it a test too. > > Thanks for the guidance. > Just sent and tested it for 5.4 kernel, please review it once. This is not the correct way to submit patches for inclusion in the stable kernel tree. Please read: https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html for how to do this properly.
Re: [PATCH v1] trace: Fix race in trace_open and buffer resize call
On 1/24/2021 8:51 AM, Steven Rostedt wrote: On Sat, 23 Jan 2021 22:03:27 +0530 Gaurav Kohli wrote: Sure I will do, I have never posted on backport branches. Let me check and post it. Basically you take your original patch that was in mainline (as the subject and commit message), and make it work as if you were doing the same exact fix for the stable release. Send it to me (and Cc everyone else), and I'll give it a test too. Thanks for the guidance. Just sent and tested it for 5.4 kernel, please review it once. Thanks! -- Steve -- Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
Re: [PATCH v1] trace: Fix race in trace_open and buffer resize call
On Sat, 23 Jan 2021 22:03:27 +0530 Gaurav Kohli wrote: > Sure I will do, I have never posted on backport branches. Let me check > and post it. > Basically you take your original patch that was in mainline (as the subject and commit message), and make it work as if you were doing the same exact fix for the stable release. Send it to me (and Cc everyone else), and I'll give it a test too. Thanks! -- Steve
Re: [PATCH v1] trace: Fix race in trace_open and buffer resize call
On 1/23/2021 4:19 PM, Denis Efremov wrote: On 1/22/21 5:37 PM, Steven Rostedt wrote: On Fri, 22 Jan 2021 16:55:29 +0530 Gaurav Kohli wrote: That could possibly work. Yes, this will work, As i have tested similar patch for internal testing for kernel branches like 5.4/4.19. Can you or Denis send a proper patch for Greg to backport? I'll review it, test it and give my ack to it, so Greg can take it without issue. I can prepare the patch, but it will be compile-tested only from my side. Honestly, I think it's better when the patch and its backports have the same author and commit message. And I can't test the fix by myself as I don't know how to reproduce conditions for the bug. I think it's better if Gaurav will prepare this backport, unless he have reasons for me to do it or maybe just don't have enough time nowadays. Gaurav, if you want to somehow mention me you add my Reported-by: Thanks, Denis Sure I will do, I have never posted on backport branches. Let me check and post it. -- Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
Re: [PATCH v1] trace: Fix race in trace_open and buffer resize call
On 1/22/21 5:37 PM, Steven Rostedt wrote: > On Fri, 22 Jan 2021 16:55:29 +0530 > Gaurav Kohli wrote: > That could possibly work. >> >> Yes, this will work, As i have tested similar patch for internal testing >> for kernel branches like 5.4/4.19. > > Can you or Denis send a proper patch for Greg to backport? I'll review it, > test it and give my ack to it, so Greg can take it without issue. > I can prepare the patch, but it will be compile-tested only from my side. Honestly, I think it's better when the patch and its backports have the same author and commit message. And I can't test the fix by myself as I don't know how to reproduce conditions for the bug. I think it's better if Gaurav will prepare this backport, unless he have reasons for me to do it or maybe just don't have enough time nowadays. Gaurav, if you want to somehow mention me you add my Reported-by: Thanks, Denis
Re: [PATCH v1] trace: Fix race in trace_open and buffer resize call
On Fri, 22 Jan 2021 16:55:29 +0530 Gaurav Kohli wrote: > >> That could possibly work. > > Yes, this will work, As i have tested similar patch for internal testing > for kernel branches like 5.4/4.19. Can you or Denis send a proper patch for Greg to backport? I'll review it, test it and give my ack to it, so Greg can take it without issue. Thanks! -- Steve > > > > > Ok, so what can I do here? Can someone resend this as a backport to the > > other stable kernels in this way so that I can queue it up? > >
Re: [PATCH v1] trace: Fix race in trace_open and buffer resize call
On Thu, Jan 21, 2021 at 03:37:32PM -0500, Steven Rostedt wrote:
> On Thu, 21 Jan 2021 23:15:22 +0300
> Denis Efremov wrote:
>
> > On 1/21/21 10:09 PM, Steven Rostedt wrote:
> > > On Thu, 21 Jan 2021 17:30:40 +0300
> > > Denis Efremov wrote:
> > >
> > >> Hi,
> > >>
> > >> This patch (CVE-2020-27825) was tagged with
> > >> Fixes: b23d7a5f4a07a ("ring-buffer: speed up buffer resets by avoiding
> > >> synchronize_rcu for each CPU")
> > >>
> > >> I'm not an expert here but it seems like b23d7a5f4a07a only refactored
> > >> ring_buffer_reset_cpu() by introducing reset_disabled_cpu_buffer()
> > >> without
> > >> significant changes. Hence,
> > >> mutex_lock(&buffer->mutex)/mutex_unlock(&buffer->mutex)
> > >> can be backported further than b23d7a5f4a07a~ and to all LTS kernels. Is
> > >> b23d7a5f4a07a the actual cause of the bug?
> > >>
> > >
> > > Ug, that looks to be a mistake. Looking back at the thread about this:
> > >
> > >
> > > https://lore.kernel.org/linux-arm-msm/20200915141304.41fa7...@gandalf.local.home/
> > >
> >
> > I see from the link that it was planned to backport the patch to LTS
> > kernels:
> >
> > > Actually we are seeing issue in older kernel like 4.19/4.14/5.4 and there
> > > below patch was not
> > > present in stable branches:
> > > Commit b23d7a5f4a07 ("ring-buffer: speed up buffer resets by avoiding
> > > synchronize_rcu for each CPU")
> >
> > The point is that it's not backported yet. Maybe because of Fixes tag. I've
> > discovered
> > this while trying to formalize CVE-2020-27825 bug in cvehound
> > https://github.com/evdenis/cvehound/blob/master/cvehound/cve/CVE-2020-27825.cocci
> >
> > I think that the backport to the 4.4+ should be something like:
> >
> > diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> > index 547a3a5ac57b..2171b377bbc1 100644
> > --- a/kernel/trace/ring_buffer.c
> > +++ b/kernel/trace/ring_buffer.c
> > @@ -4295,6 +4295,8 @@ void ring_buffer_reset_cpu(struct ring_buffer
> > *buffer, int cpu)
> > if (!cpumask_test_cpu(cpu, buffer->cpumask))
> > return;
> >
> > + mutex_lock(&buffer->mutex);
> > +
> > atomic_inc(&buffer->resize_disabled);
> > atomic_inc(&cpu_buffer->record_disabled);
> >
> > @@ -4317,6 +4319,8 @@ void ring_buffer_reset_cpu(struct ring_buffer
> > *buffer, int cpu)
> >
> > atomic_dec(&cpu_buffer->record_disabled);
> > atomic_dec(&buffer->resize_disabled);
> > +
> > + mutex_unlock(&buffer->mutex);
> > }
> > EXPORT_SYMBOL_GPL(ring_buffer_reset_cpu);
> >
>
> That could possibly work.
Ok, so what can I do here? Can someone resend this as a backport to the
other stable kernels in this way so that I can queue it up?
thanks,
greg k-h
Re: [PATCH v1] trace: Fix race in trace_open and buffer resize call
On 1/22/2021 4:29 PM, Greg KH wrote:
On Thu, Jan 21, 2021 at 03:37:32PM -0500, Steven Rostedt wrote:
On Thu, 21 Jan 2021 23:15:22 +0300
Denis Efremov wrote:
On 1/21/21 10:09 PM, Steven Rostedt wrote:
On Thu, 21 Jan 2021 17:30:40 +0300
Denis Efremov wrote:
Hi,
This patch (CVE-2020-27825) was tagged with
Fixes: b23d7a5f4a07a ("ring-buffer: speed up buffer resets by avoiding
synchronize_rcu for each CPU")
I'm not an expert here but it seems like b23d7a5f4a07a only refactored
ring_buffer_reset_cpu() by introducing reset_disabled_cpu_buffer() without
significant changes. Hence,
mutex_lock(&buffer->mutex)/mutex_unlock(&buffer->mutex)
can be backported further than b23d7a5f4a07a~ and to all LTS kernels. Is
b23d7a5f4a07a the actual cause of the bug?
Ug, that looks to be a mistake. Looking back at the thread about this:
https://lore.kernel.org/linux-arm-msm/20200915141304.41fa7...@gandalf.local.home/
I see from the link that it was planned to backport the patch to LTS kernels:
Actually we are seeing issue in older kernel like 4.19/4.14/5.4 and there below
patch was not
present in stable branches:
Commit b23d7a5f4a07 ("ring-buffer: speed up buffer resets by avoiding
synchronize_rcu for each CPU")
The point is that it's not backported yet. Maybe because of Fixes tag. I've
discovered
this while trying to formalize CVE-2020-27825 bug in cvehound
https://github.com/evdenis/cvehound/blob/master/cvehound/cve/CVE-2020-27825.cocci
I think that the backport to the 4.4+ should be something like:
diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 547a3a5ac57b..2171b377bbc1 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -4295,6 +4295,8 @@ void ring_buffer_reset_cpu(struct ring_buffer *buffer,
int cpu)
if (!cpumask_test_cpu(cpu, buffer->cpumask))
return;
+ mutex_lock(&buffer->mutex);
+
atomic_inc(&buffer->resize_disabled);
atomic_inc(&cpu_buffer->record_disabled);
@@ -4317,6 +4319,8 @@ void ring_buffer_reset_cpu(struct ring_buffer *buffer, int cpu)
atomic_dec(&cpu_buffer->record_disabled);
atomic_dec(&buffer->resize_disabled);
+
+ mutex_unlock(&buffer->mutex);
}
EXPORT_SYMBOL_GPL(ring_buffer_reset_cpu);
That could possibly work.
Yes, this will work, As i have tested similar patch for internal testing
for kernel branches like 5.4/4.19.
Ok, so what can I do here? Can someone resend this as a backport to the
other stable kernels in this way so that I can queue it up?
thanks,
greg k-h
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center,
Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.
Re: [PATCH v1] trace: Fix race in trace_open and buffer resize call
On Thu, 21 Jan 2021 23:15:22 +0300
Denis Efremov wrote:
> On 1/21/21 10:09 PM, Steven Rostedt wrote:
> > On Thu, 21 Jan 2021 17:30:40 +0300
> > Denis Efremov wrote:
> >
> >> Hi,
> >>
> >> This patch (CVE-2020-27825) was tagged with
> >> Fixes: b23d7a5f4a07a ("ring-buffer: speed up buffer resets by avoiding
> >> synchronize_rcu for each CPU")
> >>
> >> I'm not an expert here but it seems like b23d7a5f4a07a only refactored
> >> ring_buffer_reset_cpu() by introducing reset_disabled_cpu_buffer() without
> >> significant changes. Hence,
> >> mutex_lock(&buffer->mutex)/mutex_unlock(&buffer->mutex)
> >> can be backported further than b23d7a5f4a07a~ and to all LTS kernels. Is
> >> b23d7a5f4a07a the actual cause of the bug?
> >>
> >
> > Ug, that looks to be a mistake. Looking back at the thread about this:
> >
> >
> > https://lore.kernel.org/linux-arm-msm/20200915141304.41fa7...@gandalf.local.home/
> >
>
> I see from the link that it was planned to backport the patch to LTS kernels:
>
> > Actually we are seeing issue in older kernel like 4.19/4.14/5.4 and there
> > below patch was not
> > present in stable branches:
> > Commit b23d7a5f4a07 ("ring-buffer: speed up buffer resets by avoiding
> > synchronize_rcu for each CPU")
>
> The point is that it's not backported yet. Maybe because of Fixes tag. I've
> discovered
> this while trying to formalize CVE-2020-27825 bug in cvehound
> https://github.com/evdenis/cvehound/blob/master/cvehound/cve/CVE-2020-27825.cocci
>
> I think that the backport to the 4.4+ should be something like:
>
> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> index 547a3a5ac57b..2171b377bbc1 100644
> --- a/kernel/trace/ring_buffer.c
> +++ b/kernel/trace/ring_buffer.c
> @@ -4295,6 +4295,8 @@ void ring_buffer_reset_cpu(struct ring_buffer *buffer,
> int cpu)
> if (!cpumask_test_cpu(cpu, buffer->cpumask))
> return;
>
> + mutex_lock(&buffer->mutex);
> +
> atomic_inc(&buffer->resize_disabled);
> atomic_inc(&cpu_buffer->record_disabled);
>
> @@ -4317,6 +4319,8 @@ void ring_buffer_reset_cpu(struct ring_buffer *buffer,
> int cpu)
>
> atomic_dec(&cpu_buffer->record_disabled);
> atomic_dec(&buffer->resize_disabled);
> +
> + mutex_unlock(&buffer->mutex);
> }
> EXPORT_SYMBOL_GPL(ring_buffer_reset_cpu);
>
That could possibly work.
-- Steve
Re: [PATCH v1] trace: Fix race in trace_open and buffer resize call
On 1/21/21 10:09 PM, Steven Rostedt wrote:
> On Thu, 21 Jan 2021 17:30:40 +0300
> Denis Efremov wrote:
>
>> Hi,
>>
>> This patch (CVE-2020-27825) was tagged with
>> Fixes: b23d7a5f4a07a ("ring-buffer: speed up buffer resets by avoiding
>> synchronize_rcu for each CPU")
>>
>> I'm not an expert here but it seems like b23d7a5f4a07a only refactored
>> ring_buffer_reset_cpu() by introducing reset_disabled_cpu_buffer() without
>> significant changes. Hence,
>> mutex_lock(&buffer->mutex)/mutex_unlock(&buffer->mutex)
>> can be backported further than b23d7a5f4a07a~ and to all LTS kernels. Is
>> b23d7a5f4a07a the actual cause of the bug?
>>
>
> Ug, that looks to be a mistake. Looking back at the thread about this:
>
>
> https://lore.kernel.org/linux-arm-msm/20200915141304.41fa7...@gandalf.local.home/
I see from the link that it was planned to backport the patch to LTS kernels:
> Actually we are seeing issue in older kernel like 4.19/4.14/5.4 and there
> below patch was not
> present in stable branches:
> Commit b23d7a5f4a07 ("ring-buffer: speed up buffer resets by avoiding
> synchronize_rcu for each CPU")
The point is that it's not backported yet. Maybe because of Fixes tag. I've
discovered
this while trying to formalize CVE-2020-27825 bug in cvehound
https://github.com/evdenis/cvehound/blob/master/cvehound/cve/CVE-2020-27825.cocci
I think that the backport to the 4.4+ should be something like:
diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 547a3a5ac57b..2171b377bbc1 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -4295,6 +4295,8 @@ void ring_buffer_reset_cpu(struct ring_buffer *buffer,
int cpu)
if (!cpumask_test_cpu(cpu, buffer->cpumask))
return;
+ mutex_lock(&buffer->mutex);
+
atomic_inc(&buffer->resize_disabled);
atomic_inc(&cpu_buffer->record_disabled);
@@ -4317,6 +4319,8 @@ void ring_buffer_reset_cpu(struct ring_buffer *buffer,
int cpu)
atomic_dec(&cpu_buffer->record_disabled);
atomic_dec(&buffer->resize_disabled);
+
+ mutex_unlock(&buffer->mutex);
}
EXPORT_SYMBOL_GPL(ring_buffer_reset_cpu);
Thanks,
Denis
Re: [PATCH v1] trace: Fix race in trace_open and buffer resize call
On Thu, 21 Jan 2021 17:30:40 +0300
Denis Efremov wrote:
> Hi,
>
> This patch (CVE-2020-27825) was tagged with
> Fixes: b23d7a5f4a07a ("ring-buffer: speed up buffer resets by avoiding
> synchronize_rcu for each CPU")
>
> I'm not an expert here but it seems like b23d7a5f4a07a only refactored
> ring_buffer_reset_cpu() by introducing reset_disabled_cpu_buffer() without
> significant changes. Hence,
> mutex_lock(&buffer->mutex)/mutex_unlock(&buffer->mutex)
> can be backported further than b23d7a5f4a07a~ and to all LTS kernels. Is
> b23d7a5f4a07a the actual cause of the bug?
>
Ug, that looks to be a mistake. Looking back at the thread about this:
https://lore.kernel.org/linux-arm-msm/20200915141304.41fa7...@gandalf.local.home/
That should have been:
Depends-on: b23d7a5f4a07 ("ring-buffer: speed up buffer resets by avoiding
synchronize_rcu for each CPU")
-- Steve
Re: [PATCH v1] trace: Fix race in trace_open and buffer resize call
Hi,
This patch (CVE-2020-27825) was tagged with
Fixes: b23d7a5f4a07a ("ring-buffer: speed up buffer resets by avoiding
synchronize_rcu for each CPU")
I'm not an expert here but it seems like b23d7a5f4a07a only refactored
ring_buffer_reset_cpu() by introducing reset_disabled_cpu_buffer() without
significant changes. Hence,
mutex_lock(&buffer->mutex)/mutex_unlock(&buffer->mutex)
can be backported further than b23d7a5f4a07a~ and to all LTS kernels. Is
b23d7a5f4a07a the actual cause of the bug?
Thanks,
Denis
On 10/6/20 12:33 PM, Gaurav Kohli wrote:
> Below race can come, if trace_open and resize of
> cpu buffer is running parallely on different cpus
> CPUXCPUY
> ring_buffer_resize
> atomic_read(&buffer->resize_disabled)
> tracing_open
> tracing_reset_online_cpus
> ring_buffer_reset_cpu
> rb_reset_cpu
> rb_update_pages
> remove/insert pages
> resetting pointer
>
> This race can cause data abort or some times infinte loop in
> rb_remove_pages and rb_insert_pages while checking pages
> for sanity.
>
> Take buffer lock to fix this.
>
> Signed-off-by: Gaurav Kohli
> Cc: sta...@vger.kernel.org
> ---
> Changes since v0:
> -Addressed Steven's review comments.
>
> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> index 93ef0ab..15bf28b 100644
> --- a/kernel/trace/ring_buffer.c
> +++ b/kernel/trace/ring_buffer.c
> @@ -4866,6 +4866,9 @@ void ring_buffer_reset_cpu(struct trace_buffer *buffer,
> int cpu)
> if (!cpumask_test_cpu(cpu, buffer->cpumask))
> return;
>
> + /* prevent another thread from changing buffer sizes */
> + mutex_lock(&buffer->mutex);
> +
> atomic_inc(&cpu_buffer->resize_disabled);
> atomic_inc(&cpu_buffer->record_disabled);
>
> @@ -4876,6 +4879,8 @@ void ring_buffer_reset_cpu(struct trace_buffer *buffer,
> int cpu)
>
> atomic_dec(&cpu_buffer->record_disabled);
> atomic_dec(&cpu_buffer->resize_disabled);
> +
> + mutex_unlock(&buffer->mutex);
> }
> EXPORT_SYMBOL_GPL(ring_buffer_reset_cpu);
>
> @@ -4889,6 +4894,9 @@ void ring_buffer_reset_online_cpus(struct trace_buffer
> *buffer)
> struct ring_buffer_per_cpu *cpu_buffer;
> int cpu;
>
> + /* prevent another thread from changing buffer sizes */
> + mutex_lock(&buffer->mutex);
> +
> for_each_online_buffer_cpu(buffer, cpu) {
> cpu_buffer = buffer->buffers[cpu];
>
> @@ -4907,6 +4915,8 @@ void ring_buffer_reset_online_cpus(struct trace_buffer
> *buffer)
> atomic_dec(&cpu_buffer->record_disabled);
> atomic_dec(&cpu_buffer->resize_disabled);
> }
> +
> + mutex_unlock(&buffer->mutex);
> }
>
> /**
>
[PATCH v1] trace: Fix race in trace_open and buffer resize call
Below race can come, if trace_open and resize of
cpu buffer is running parallely on different cpus
CPUXCPUY
ring_buffer_resize
atomic_read(&buffer->resize_disabled)
tracing_open
tracing_reset_online_cpus
ring_buffer_reset_cpu
rb_reset_cpu
rb_update_pages
remove/insert pages
resetting pointer
This race can cause data abort or some times infinte loop in
rb_remove_pages and rb_insert_pages while checking pages
for sanity.
Take buffer lock to fix this.
Signed-off-by: Gaurav Kohli
Cc: sta...@vger.kernel.org
---
Changes since v0:
-Addressed Steven's review comments.
diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 93ef0ab..15bf28b 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -4866,6 +4866,9 @@ void ring_buffer_reset_cpu(struct trace_buffer *buffer,
int cpu)
if (!cpumask_test_cpu(cpu, buffer->cpumask))
return;
+ /* prevent another thread from changing buffer sizes */
+ mutex_lock(&buffer->mutex);
+
atomic_inc(&cpu_buffer->resize_disabled);
atomic_inc(&cpu_buffer->record_disabled);
@@ -4876,6 +4879,8 @@ void ring_buffer_reset_cpu(struct trace_buffer *buffer,
int cpu)
atomic_dec(&cpu_buffer->record_disabled);
atomic_dec(&cpu_buffer->resize_disabled);
+
+ mutex_unlock(&buffer->mutex);
}
EXPORT_SYMBOL_GPL(ring_buffer_reset_cpu);
@@ -4889,6 +4894,9 @@ void ring_buffer_reset_online_cpus(struct trace_buffer
*buffer)
struct ring_buffer_per_cpu *cpu_buffer;
int cpu;
+ /* prevent another thread from changing buffer sizes */
+ mutex_lock(&buffer->mutex);
+
for_each_online_buffer_cpu(buffer, cpu) {
cpu_buffer = buffer->buffers[cpu];
@@ -4907,6 +4915,8 @@ void ring_buffer_reset_online_cpus(struct trace_buffer
*buffer)
atomic_dec(&cpu_buffer->record_disabled);
atomic_dec(&cpu_buffer->resize_disabled);
}
+
+ mutex_unlock(&buffer->mutex);
}
/**
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center,
Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative
Project
Re: [PATCH v1] trace: Fix race in trace_open and buffer resize call
On 10/5/2020 10:02 PM, Steven Rostedt wrote: On Mon, 5 Oct 2020 21:59:02 +0530 Gaurav Kohli wrote: Hi Steven, I am using normal git send-email(never saw problem with this), Not sure what is wrong. In my older mail i have kept you in to and rest in cc. Let me try to resent it. The Cc is working (I got it in my LKML box), but I don't see any connection in my server. Note, the rost...@goodmis.org is a server at my home. -- Steve Thanks for update, i will verify my account settings and will send my patch tomorrow. -- Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
Re: [PATCH v1] trace: Fix race in trace_open and buffer resize call
On Mon, 5 Oct 2020 21:59:02 +0530 Gaurav Kohli wrote: > Hi Steven, > > I am using normal git send-email(never saw problem with this), Not sure > what is wrong. In my older mail i have kept you in to and rest in cc. > > Let me try to resent it. The Cc is working (I got it in my LKML box), but I don't see any connection in my server. Note, the rost...@goodmis.org is a server at my home. -- Steve
Re: [PATCH v1] trace: Fix race in trace_open and buffer resize call
On 10/5/2020 7:57 PM, Steven Rostedt wrote: On Mon, 5 Oct 2020 10:25:15 -0400 Steven Rostedt wrote: On Mon, 5 Oct 2020 10:09:34 +0530 Gaurav Kohli wrote: Hi Steven, please let us know, if below looks good to you or need modifications. Strange, I don't have your original email in my inbox. I do have it in my LKML folder, but that's way too big for me to read. I checked my server logs. I found where I received this from LKML, but there's no log that I received this directly. How are you sending out your patches? If it doesn't land in my inbox, I'll never see it. BTW, this is the second time this has happened to me. The first time I assumed it may have been me accidentally deleting it. Now I'm thinking it's on your end. I have yet to received a patch from you directly. Last time I had to pull it from my LKML folder after you replied to me (letting me know you sent it). -- Steve Hi Steven, I am using normal git send-email(never saw problem with this), Not sure what is wrong. In my older mail i have kept you in to and rest in cc. Let me try to resent it. -- Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
Re: [PATCH v1] trace: Fix race in trace_open and buffer resize call
On Mon, 5 Oct 2020 10:25:15 -0400 Steven Rostedt wrote: > On Mon, 5 Oct 2020 10:09:34 +0530 > Gaurav Kohli wrote: > > > Hi Steven, > > > > please let us know, if below looks good to you or need modifications. > > Strange, I don't have your original email in my inbox. I do have it in my > LKML folder, but that's way too big for me to read. I checked my server > logs. I found where I received this from LKML, but there's no log that I > received this directly. > > How are you sending out your patches? If it doesn't land in my inbox, I'll > never see it. > BTW, this is the second time this has happened to me. The first time I assumed it may have been me accidentally deleting it. Now I'm thinking it's on your end. I have yet to received a patch from you directly. Last time I had to pull it from my LKML folder after you replied to me (letting me know you sent it). -- Steve
Re: [PATCH v1] trace: Fix race in trace_open and buffer resize call
On Mon, 5 Oct 2020 10:09:34 +0530 Gaurav Kohli wrote: > Hi Steven, > > please let us know, if below looks good to you or need modifications. Strange, I don't have your original email in my inbox. I do have it in my LKML folder, but that's way too big for me to read. I checked my server logs. I found where I received this from LKML, but there's no log that I received this directly. How are you sending out your patches? If it doesn't land in my inbox, I'll never see it. -- Steve
Re: [PATCH v1] trace: Fix race in trace_open and buffer resize call
Hi Steven,
please let us know, if below looks good to you or need modifications.
Thanks
Gaurav
On 9/24/2020 7:25 PM, Gaurav Kohli wrote:
Below race can come, if trace_open and resize of
cpu buffer is running parallely on different cpus
CPUXCPUY
ring_buffer_resize
atomic_read(&buffer->resize_disabled)
tracing_open
tracing_reset_online_cpus
ring_buffer_reset_cpu
rb_reset_cpu
rb_update_pages
remove/insert pages
resetting pointer
This race can cause data abort or some times infinte loop in
rb_remove_pages and rb_insert_pages while checking pages
for sanity.
Signed-off-by: Gaurav Kohli
Cc: sta...@vger.kernel.org
---
Changes since v0:
-Addressed Steven's review comments.
diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 93ef0ab..15bf28b 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -4866,6 +4866,9 @@ void ring_buffer_reset_cpu(struct trace_buffer *buffer,
int cpu)
if (!cpumask_test_cpu(cpu, buffer->cpumask))
return;
+ /* prevent another thread from changing buffer sizes */
+ mutex_lock(&buffer->mutex);
+
atomic_inc(&cpu_buffer->resize_disabled);
atomic_inc(&cpu_buffer->record_disabled);
@@ -4876,6 +4879,8 @@ void ring_buffer_reset_cpu(struct trace_buffer *buffer, int cpu)
atomic_dec(&cpu_buffer->record_disabled);
atomic_dec(&cpu_buffer->resize_disabled);
+
+ mutex_unlock(&buffer->mutex);
}
EXPORT_SYMBOL_GPL(ring_buffer_reset_cpu);
@@ -4889,6 +4894,9 @@ void ring_buffer_reset_online_cpus(struct trace_buffer *buffer)
struct ring_buffer_per_cpu *cpu_buffer;
int cpu;
+ /* prevent another thread from changing buffer sizes */
+ mutex_lock(&buffer->mutex);
+
for_each_online_buffer_cpu(buffer, cpu) {
cpu_buffer = buffer->buffers[cpu];
@@ -4907,6 +4915,8 @@ void ring_buffer_reset_online_cpus(struct trace_buffer *buffer)
atomic_dec(&cpu_buffer->record_disabled);
atomic_dec(&cpu_buffer->resize_disabled);
}
+
+ mutex_unlock(&buffer->mutex);
}
/**
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center,
Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.
[PATCH v1] trace: Fix race in trace_open and buffer resize call
Below race can come, if trace_open and resize of
cpu buffer is running parallely on different cpus
CPUXCPUY
ring_buffer_resize
atomic_read(&buffer->resize_disabled)
tracing_open
tracing_reset_online_cpus
ring_buffer_reset_cpu
rb_reset_cpu
rb_update_pages
remove/insert pages
resetting pointer
This race can cause data abort or some times infinte loop in
rb_remove_pages and rb_insert_pages while checking pages
for sanity.
Signed-off-by: Gaurav Kohli
Cc: sta...@vger.kernel.org
---
Changes since v0:
-Addressed Steven's review comments.
diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 93ef0ab..15bf28b 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -4866,6 +4866,9 @@ void ring_buffer_reset_cpu(struct trace_buffer *buffer,
int cpu)
if (!cpumask_test_cpu(cpu, buffer->cpumask))
return;
+ /* prevent another thread from changing buffer sizes */
+ mutex_lock(&buffer->mutex);
+
atomic_inc(&cpu_buffer->resize_disabled);
atomic_inc(&cpu_buffer->record_disabled);
@@ -4876,6 +4879,8 @@ void ring_buffer_reset_cpu(struct trace_buffer *buffer,
int cpu)
atomic_dec(&cpu_buffer->record_disabled);
atomic_dec(&cpu_buffer->resize_disabled);
+
+ mutex_unlock(&buffer->mutex);
}
EXPORT_SYMBOL_GPL(ring_buffer_reset_cpu);
@@ -4889,6 +4894,9 @@ void ring_buffer_reset_online_cpus(struct trace_buffer
*buffer)
struct ring_buffer_per_cpu *cpu_buffer;
int cpu;
+ /* prevent another thread from changing buffer sizes */
+ mutex_lock(&buffer->mutex);
+
for_each_online_buffer_cpu(buffer, cpu) {
cpu_buffer = buffer->buffers[cpu];
@@ -4907,6 +4915,8 @@ void ring_buffer_reset_online_cpus(struct trace_buffer
*buffer)
atomic_dec(&cpu_buffer->record_disabled);
atomic_dec(&cpu_buffer->resize_disabled);
}
+
+ mutex_unlock(&buffer->mutex);
}
/**
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center,
Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative
Project
