LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] Don't risk NULL deref in marker
@ 2008-02-25 23:14 Jesper Juhl
  2008-02-26  2:59 ` Mathieu Desnoyers
  0 siblings, 1 reply; 2+ messages in thread
From: Jesper Juhl @ 2008-02-25 23:14 UTC (permalink / raw)
  To: Mathieu Desnoyers; +Cc: LKML, Jesper Juhl


get_marker() may return NULL, so test for it.


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;

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] Don't risk NULL deref in marker
  2008-02-25 23:14 [PATCH] Don't risk NULL deref in marker Jesper Juhl
@ 2008-02-26  2:59 ` Mathieu Desnoyers
  0 siblings, 0 replies; 2+ messages in thread
From: Mathieu Desnoyers @ 2008-02-26  2:59 UTC (permalink / raw)
  To: Jesper Juhl; +Cc: LKML

* 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

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2008-02-26  2:59 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-02-25 23:14 [PATCH] Don't risk NULL deref in marker Jesper Juhl
2008-02-26  2:59 ` Mathieu Desnoyers

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).