Re: [PATCH] Markers : fix check format with rcu callback race

From: Mathieu Desnoyers
Date: Fri Oct 10 2008 - 03:23:21 EST


* Lai Jiangshan (laijs@xxxxxxxxxxxxxx) 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@xxxxxxxxxxxxxx) 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@xxxxxxxxxx>
> > CC: Ingo Molnar <mingo@xxxxxxx>
> > CC: Lai Jiangshan <laijs@xxxxxxxxxxxxxx>
> > ---
> > 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/