[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20060914140459.GA23823@sergelap.austin.ibm.com>
Date: Thu, 14 Sep 2006 09:04:59 -0500
From: "Serge E. Hallyn" <serue@...ibm.com>
To: Mathieu Desnoyers <mathieu.desnoyers@...ymtl.ca>
Cc: linux-kernel@...r.kernel.org,
Christoph Hellwig <hch@...radead.org>,
Andrew Morton <akpm@...l.org>, Ingo Molnar <mingo@...hat.com>,
Greg Kroah-Hartman <gregkh@...e.de>,
Thomas Gleixner <tglx@...utronix.de>,
Tom Zanussi <zanussi@...ibm.com>, ltt-dev@...fik.org,
Michel Dagenais <michel.dagenais@...ymtl.ca>
Subject: Re: [PATCH 4/11] LTTng-core 0.5.108 : core
Hi,
I'm wondering why this is safe:
Quoting Mathieu Desnoyers (mathieu.desnoyers@...ymtl.ca):
...
> +void ltt_transport_register(struct ltt_transport *transport)
> +{
> + spin_lock(&transport_list_lock);
> + list_add_tail(&transport->node, <t_transport_list);
> + spin_unlock(&transport_list_lock);
> +}
> +
> +void ltt_transport_unregister(struct ltt_transport *transport)
> +{
> + spin_lock(&transport_list_lock);
> + list_del(&transport->node);
You don't have a refcount you check here, and
> + spin_unlock(&transport_list_lock);
> +}
> +
> +EXPORT_SYMBOL_GPL(ltt_transport_register);
> +EXPORT_SYMBOL_GPL(ltt_transport_unregister);
...
> +static int ltt_trace_create(char *trace_name, char *trace_type,
> + enum trace_mode mode,
> + unsigned subbuf_size_low, unsigned n_subbufs_low,
> + unsigned subbuf_size_med, unsigned n_subbufs_med,
> + unsigned subbuf_size_high, unsigned n_subbufs_high)
> +{
here:
> + spin_lock(&transport_list_lock);
> + list_for_each_entry(tran, <t_transport_list, node) {
> + if (!strcmp(tran->name, trace_type)) {
> + transport = tran;
> + break;
> + }
> + }
> + spin_unlock(&transport_list_lock);
> +
> + if (!transport) {
> + err = EINVAL;
> + printk(KERN_ERR "LTT : Transport %s is not present.\n", trace_type);
> + goto trace_error;
> + }
> +
> + if(!try_module_get(transport->owner)) {
> + err = ENODEV;
> + printk(KERN_ERR "LTT : Can't lock transport module.\n");
> + goto trace_error;
> + }
> +
> + new_trace->transport = transport;
> + new_trace->ops = &transport->ops;
you grab references to the object which may be deleted after
you drop the transport_list_lock at the top of this block. Since
a later patch shows the unregister being called right before the
owning module is unloaded, that seems awefuly dangerous.
Is there some other magic going on making this safe?
thanks,
-serge
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists