Re: [PATCH] Don't risk NULL deref in marker

[Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>]

* Jesper Juhl (jesper.juhl@gmail.com) wrote:
> 
> get_marker() may return NULL, so test for it.
> 

Hrm, yes, if we have two marker_probe_unregister callers calling it for
the same marker, one expecting it to fail and they race, yes, it can
happen. Although this is not expected to happen often if the caller acts
sanely, it's a good thing to fix it. Thanks!

Acked-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>

> 
> Signed-off-by: Jesper Juhl <jesper.juhl@gmail.com>
> ---
> 
> diff --git a/kernel/marker.c b/kernel/marker.c
> index 50effc0..f211f08 100644
> --- a/kernel/marker.c
> +++ b/kernel/marker.c
> @@ -698,12 +698,11 @@ int marker_probe_unregister(const char *name,
>  {
>  	struct marker_entry *entry;
>  	struct marker_probe_closure *old;
> -	int ret = 0;
> +	int ret = -ENOENT;
>  
>  	mutex_lock(&markers_mutex);
>  	entry = get_marker(name);
>  	if (!entry) {
> -		ret = -ENOENT;
>  		goto end;
>  	}
>  	if (entry->rcu_pending)
> @@ -713,12 +712,16 @@ int marker_probe_unregister(const char *name,
>  	marker_update_probes();		/* may update entry */
>  	mutex_lock(&markers_mutex);
>  	entry = get_marker(name);
> +	if (!entry) {
> +		goto end;
> +	}
>  	entry->oldptr = old;
>  	entry->rcu_pending = 1;
>  	/* write rcu_pending before calling the RCU callback */
>  	smp_wmb();
>  	call_rcu(&entry->rcu, free_old_closure);
>  	remove_marker(name);	/* Ignore busy error message */
> +	ret = 0;
>  end:
>  	mutex_unlock(&markers_mutex);
>  	return ret;

-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68