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

[Lai Jiangshan <laijs@cn.fujitsu.com>]

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);