* [PATCH] Markers : fix check format with rcu callback race
@ 2008-10-10 5:44 Mathieu Desnoyers
2008-10-10 6:18 ` Lai Jiangshan
0 siblings, 1 reply; 3+ messages in thread
From: Mathieu Desnoyers @ 2008-10-10 5:44 UTC (permalink / raw)
To: Ingo Molnar; +Cc: Lai Jiangshan, linux-kernel
The fix "markers: fix unchecked format" introduced an RCU callback race. This
patch takes care of calling any pending RCU callback before set_format is
called.
* Lai Jiangshan (laijs@cn.fujitsu.com) wrote:
>
> bit-field is not thread-safe nor smp-safe.
>
> struct marker_entry.rcu_pending is not protected by any lock
> in rcu-callback free_old_closure().
> so we must turn it into a safe type.
>
All struct marker_entry.rcu_pending accesses are done with the
markers_mutex held, except the one done in free_old_closure(). Normally,
there should be a
if (entry->rcu_pending)
rcu_barrier_sched();
At the beginning of each markers_mutex section (just after get_marker())
to make sure any pending callback is executed at that point before any
of rcu_pending or ptype are touched.
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
CC: Ingo Molnar <mingo@elte.hu>
CC: Lai Jiangshan <laijs@cn.fujitsu.com>
---
kernel/marker.c | 24 +++++++++++++-----------
1 file changed, 13 insertions(+), 11 deletions(-)
Index: linux-2.6-lttng/kernel/marker.c
===================================================================
--- linux-2.6-lttng.orig/kernel/marker.c 2008-10-10 01:35:32.000000000 -0400
+++ linux-2.6-lttng/kernel/marker.c 2008-10-10 01:35:32.000000000 -0400
@@ -657,21 +657,23 @@ int marker_probe_register(const char *na
entry = add_marker(name, format);
if (IS_ERR(entry))
ret = PTR_ERR(entry);
- } else if (format) {
- if (!entry->format)
- ret = marker_set_format(&entry, format);
- else if (strcmp(entry->format, format))
- ret = -EPERM;
+ } else {
+ /*
+ * If we detect that a call_rcu is pending for this marker,
+ * make sure it's executed now.
+ */
+ if (entry->rcu_pending)
+ rcu_barrier_sched();
+ if (format) {
+ if (!entry->format)
+ ret = marker_set_format(&entry, format);
+ else if (strcmp(entry->format, format))
+ ret = -EPERM;
+ }
}
if (ret)
goto end;
- /*
- * If we detect that a call_rcu is pending for this marker,
- * make sure it's executed now.
- */
- if (entry->rcu_pending)
- rcu_barrier_sched();
old = marker_entry_add_probe(entry, probe, probe_private);
if (IS_ERR(old)) {
ret = PTR_ERR(old);
--
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] Markers : fix check format with rcu callback race
2008-10-10 5:44 [PATCH] Markers : fix check format with rcu callback race Mathieu Desnoyers
@ 2008-10-10 6:18 ` Lai Jiangshan
2008-10-10 7:23 ` Mathieu Desnoyers
0 siblings, 1 reply; 3+ messages in thread
From: Lai Jiangshan @ 2008-10-10 6:18 UTC (permalink / raw)
To: Mathieu Desnoyers; +Cc: Ingo Molnar, linux-kernel
Mathieu Desnoyers wrote:
> The fix "markers: fix unchecked format" introduced an RCU callback race. This
> patch takes care of calling any pending RCU callback before set_format is
> called.
>
marker_set_format() has this statement:
if ((*entry)->rcu_pending)
rcu_barrier_sched();
>
> * Lai Jiangshan (laijs@cn.fujitsu.com) wrote:
>> bit-field is not thread-safe nor smp-safe.
>>
>> struct marker_entry.rcu_pending is not protected by any lock
>> in rcu-callback free_old_closure().
>> so we must turn it into a safe type.
>>
>
> All struct marker_entry.rcu_pending accesses are done with the
> markers_mutex held, except the one done in free_old_closure(). Normally,
> there should be a
> if (entry->rcu_pending)
> rcu_barrier_sched();
>
> At the beginning of each markers_mutex section (just after get_marker())
> to make sure any pending callback is executed at that point before any
> of rcu_pending or ptype are touched.
>
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
> CC: Ingo Molnar <mingo@elte.hu>
> CC: Lai Jiangshan <laijs@cn.fujitsu.com>
> ---
> kernel/marker.c | 24 +++++++++++++-----------
> 1 file changed, 13 insertions(+), 11 deletions(-)
>
> Index: linux-2.6-lttng/kernel/marker.c
> ===================================================================
> --- linux-2.6-lttng.orig/kernel/marker.c 2008-10-10 01:35:32.000000000 -0400
> +++ linux-2.6-lttng/kernel/marker.c 2008-10-10 01:35:32.000000000 -0400
> @@ -657,21 +657,23 @@ int marker_probe_register(const char *na
> entry = add_marker(name, format);
> if (IS_ERR(entry))
> ret = PTR_ERR(entry);
> - } else if (format) {
> - if (!entry->format)
> - ret = marker_set_format(&entry, format);
> - else if (strcmp(entry->format, format))
> - ret = -EPERM;
> + } else {
> + /*
> + * If we detect that a call_rcu is pending for this marker,
> + * make sure it's executed now.
> + */
> + if (entry->rcu_pending)
> + rcu_barrier_sched();
> + if (format) {
> + if (!entry->format)
> + ret = marker_set_format(&entry, format);
> + else if (strcmp(entry->format, format))
> + ret = -EPERM;
> + }
> }
> if (ret)
> goto end;
>
> - /*
> - * If we detect that a call_rcu is pending for this marker,
> - * make sure it's executed now.
> - */
> - if (entry->rcu_pending)
> - rcu_barrier_sched();
> old = marker_entry_add_probe(entry, probe, probe_private);
> if (IS_ERR(old)) {
> ret = PTR_ERR(old);
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] Markers : fix check format with rcu callback race
2008-10-10 6:18 ` Lai Jiangshan
@ 2008-10-10 7:23 ` Mathieu Desnoyers
0 siblings, 0 replies; 3+ messages in thread
From: Mathieu Desnoyers @ 2008-10-10 7:23 UTC (permalink / raw)
To: Lai Jiangshan; +Cc: Ingo Molnar, linux-kernel
* Lai Jiangshan (laijs@cn.fujitsu.com) wrote:
> Mathieu Desnoyers wrote:
> > The fix "markers: fix unchecked format" introduced an RCU callback race. This
> > patch takes care of calling any pending RCU callback before set_format is
> > called.
> >
>
> marker_set_format() has this statement:
>
> if ((*entry)->rcu_pending)
> rcu_barrier_sched();
>
True, therefore my fix is not needed.
Mathieu
> >
> > * Lai Jiangshan (laijs@cn.fujitsu.com) wrote:
> >> bit-field is not thread-safe nor smp-safe.
> >>
> >> struct marker_entry.rcu_pending is not protected by any lock
> >> in rcu-callback free_old_closure().
> >> so we must turn it into a safe type.
> >>
> >
> > All struct marker_entry.rcu_pending accesses are done with the
> > markers_mutex held, except the one done in free_old_closure(). Normally,
> > there should be a
> > if (entry->rcu_pending)
> > rcu_barrier_sched();
> >
> > At the beginning of each markers_mutex section (just after get_marker())
> > to make sure any pending callback is executed at that point before any
> > of rcu_pending or ptype are touched.
> >
> > Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
> > CC: Ingo Molnar <mingo@elte.hu>
> > CC: Lai Jiangshan <laijs@cn.fujitsu.com>
> > ---
> > kernel/marker.c | 24 +++++++++++++-----------
> > 1 file changed, 13 insertions(+), 11 deletions(-)
> >
> > Index: linux-2.6-lttng/kernel/marker.c
> > ===================================================================
> > --- linux-2.6-lttng.orig/kernel/marker.c 2008-10-10 01:35:32.000000000 -0400
> > +++ linux-2.6-lttng/kernel/marker.c 2008-10-10 01:35:32.000000000 -0400
> > @@ -657,21 +657,23 @@ int marker_probe_register(const char *na
> > entry = add_marker(name, format);
> > if (IS_ERR(entry))
> > ret = PTR_ERR(entry);
> > - } else if (format) {
> > - if (!entry->format)
> > - ret = marker_set_format(&entry, format);
> > - else if (strcmp(entry->format, format))
> > - ret = -EPERM;
> > + } else {
> > + /*
> > + * If we detect that a call_rcu is pending for this marker,
> > + * make sure it's executed now.
> > + */
> > + if (entry->rcu_pending)
> > + rcu_barrier_sched();
> > + if (format) {
> > + if (!entry->format)
> > + ret = marker_set_format(&entry, format);
> > + else if (strcmp(entry->format, format))
> > + ret = -EPERM;
> > + }
> > }
> > if (ret)
> > goto end;
> >
> > - /*
> > - * If we detect that a call_rcu is pending for this marker,
> > - * make sure it's executed now.
> > - */
> > - if (entry->rcu_pending)
> > - rcu_barrier_sched();
> > old = marker_entry_add_probe(entry, probe, probe_private);
> > if (IS_ERR(old)) {
> > ret = PTR_ERR(old);
>
>
--
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2008-10-10 7:23 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-10-10 5:44 [PATCH] Markers : fix check format with rcu callback race Mathieu Desnoyers
2008-10-10 6:18 ` Lai Jiangshan
2008-10-10 7:23 ` Mathieu Desnoyers
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox