From dd5a0db3ea07c46bee3c1814ef7326736f38a06e Mon Sep 17 00:00:00 2001 From: Mathieu Desnoyers Date: Tue, 11 Feb 2014 18:18:51 -0500 Subject: [PATCH] Fix: use after free in ring buffer clients Don't use ring buffer client's struct lttng_channel from ioctl which applies to ring buffer streams, because lttng_channel is freed while lib ring buffer stream and channel are still in use. Their lifetime persists until the consumer daemon releases its handles on the related stream file descriptors. Signed-off-by: Mathieu Desnoyers --- lib/ringbuffer/backend_types.h | 2 + lib/ringbuffer/ring_buffer_frontend.c | 3 ++ lttng-abi.c | 60 +++++++-------------------- lttng-ring-buffer-client.h | 39 ++++++++++++++--- lttng-ring-buffer-metadata-client.h | 39 ++++++++++++++--- 5 files changed, 87 insertions(+), 56 deletions(-) diff --git a/lib/ringbuffer/backend_types.h b/lib/ringbuffer/backend_types.h index 6813dd8b..1577c812 100644 --- a/lib/ringbuffer/backend_types.h +++ b/lib/ringbuffer/backend_types.h @@ -83,6 +83,8 @@ struct channel_backend { unsigned long num_subbuf; /* Number of sub-buffers for writer */ u64 start_tsc; /* Channel creation TSC value */ void *priv; /* Client-specific information */ + void *priv_ops; /* Client-specific ops pointer */ + void (*release_priv_ops)(void *priv_ops); struct notifier_block cpu_hp_notifier; /* CPU hotplug notifier */ /* * We need to copy config because the module containing the diff --git a/lib/ringbuffer/ring_buffer_frontend.c b/lib/ringbuffer/ring_buffer_frontend.c index 9bb41841..fc8d5413 100644 --- a/lib/ringbuffer/ring_buffer_frontend.c +++ b/lib/ringbuffer/ring_buffer_frontend.c @@ -585,6 +585,9 @@ static void channel_unregister_notifiers(struct channel *chan) static void channel_free(struct channel *chan) { + if (chan->backend.release_priv_ops) { + chan->backend.release_priv_ops(chan->backend.priv_ops); + } channel_iterator_free(chan); channel_backend_free(&chan->backend); kfree(chan); diff --git a/lttng-abi.c b/lttng-abi.c index 1cc95101..261a0ad3 100644 --- a/lttng-abi.c +++ b/lttng-abi.c @@ -1347,7 +1347,7 @@ static long lttng_stream_ring_buffer_ioctl(struct file *filp, struct lib_ring_buffer *buf = filp->private_data; struct channel *chan = buf->backend.chan; const struct lib_ring_buffer_config *config = &chan->backend.config; - struct lttng_channel *lttng_chan = channel_get_private(chan); + const struct lttng_channel_ops *ops = chan->backend.priv_ops; int ret; if (atomic_read(&chan->record_disabled)) @@ -1358,9 +1358,7 @@ static long lttng_stream_ring_buffer_ioctl(struct file *filp, { uint64_t ts; - if (!lttng_chan->ops) - goto error; - ret = lttng_chan->ops->timestamp_begin(config, buf, &ts); + ret = ops->timestamp_begin(config, buf, &ts); if (ret < 0) goto error; return put_u64(ts, arg); @@ -1369,9 +1367,7 @@ static long lttng_stream_ring_buffer_ioctl(struct file *filp, { uint64_t ts; - if (!lttng_chan->ops) - goto error; - ret = lttng_chan->ops->timestamp_end(config, buf, &ts); + ret = ops->timestamp_end(config, buf, &ts); if (ret < 0) goto error; return put_u64(ts, arg); @@ -1380,9 +1376,7 @@ static long lttng_stream_ring_buffer_ioctl(struct file *filp, { uint64_t ed; - if (!lttng_chan->ops) - goto error; - ret = lttng_chan->ops->events_discarded(config, buf, &ed); + ret = ops->events_discarded(config, buf, &ed); if (ret < 0) goto error; return put_u64(ed, arg); @@ -1391,9 +1385,7 @@ static long lttng_stream_ring_buffer_ioctl(struct file *filp, { uint64_t cs; - if (!lttng_chan->ops) - goto error; - ret = lttng_chan->ops->content_size(config, buf, &cs); + ret = ops->content_size(config, buf, &cs); if (ret < 0) goto error; return put_u64(cs, arg); @@ -1402,9 +1394,7 @@ static long lttng_stream_ring_buffer_ioctl(struct file *filp, { uint64_t ps; - if (!lttng_chan->ops) - goto error; - ret = lttng_chan->ops->packet_size(config, buf, &ps); + ret = ops->packet_size(config, buf, &ps); if (ret < 0) goto error; return put_u64(ps, arg); @@ -1413,9 +1403,7 @@ static long lttng_stream_ring_buffer_ioctl(struct file *filp, { uint64_t si; - if (!lttng_chan->ops) - goto error; - ret = lttng_chan->ops->stream_id(config, buf, &si); + ret = ops->stream_id(config, buf, &si); if (ret < 0) goto error; return put_u64(si, arg); @@ -1424,9 +1412,7 @@ static long lttng_stream_ring_buffer_ioctl(struct file *filp, { uint64_t ts; - if (!lttng_chan->ops) - goto error; - ret = lttng_chan->ops->current_timestamp(config, buf, &ts); + ret = ops->current_timestamp(config, buf, &ts); if (ret < 0) goto error; return put_u64(ts, arg); @@ -1447,7 +1433,7 @@ static long lttng_stream_ring_buffer_compat_ioctl(struct file *filp, struct lib_ring_buffer *buf = filp->private_data; struct channel *chan = buf->backend.chan; const struct lib_ring_buffer_config *config = &chan->backend.config; - struct lttng_channel *lttng_chan = channel_get_private(chan); + const struct lttng_channel_ops *ops = chan->backend.priv_ops; int ret; if (atomic_read(&chan->record_disabled)) @@ -1458,9 +1444,7 @@ static long lttng_stream_ring_buffer_compat_ioctl(struct file *filp, { uint64_t ts; - if (!lttng_chan->ops) - goto error; - ret = lttng_chan->ops->timestamp_begin(config, buf, &ts); + ret = ops->timestamp_begin(config, buf, &ts); if (ret < 0) goto error; return put_u64(ts, arg); @@ -1469,9 +1453,7 @@ static long lttng_stream_ring_buffer_compat_ioctl(struct file *filp, { uint64_t ts; - if (!lttng_chan->ops) - goto error; - ret = lttng_chan->ops->timestamp_end(config, buf, &ts); + ret = ops->timestamp_end(config, buf, &ts); if (ret < 0) goto error; return put_u64(ts, arg); @@ -1480,9 +1462,7 @@ static long lttng_stream_ring_buffer_compat_ioctl(struct file *filp, { uint64_t ed; - if (!lttng_chan->ops) - goto error; - ret = lttng_chan->ops->events_discarded(config, buf, &ed); + ret = ops->events_discarded(config, buf, &ed); if (ret < 0) goto error; return put_u64(ed, arg); @@ -1491,9 +1471,7 @@ static long lttng_stream_ring_buffer_compat_ioctl(struct file *filp, { uint64_t cs; - if (!lttng_chan->ops) - goto error; - ret = lttng_chan->ops->content_size(config, buf, &cs); + ret = ops->content_size(config, buf, &cs); if (ret < 0) goto error; return put_u64(cs, arg); @@ -1502,9 +1480,7 @@ static long lttng_stream_ring_buffer_compat_ioctl(struct file *filp, { uint64_t ps; - if (!lttng_chan->ops) - goto error; - ret = lttng_chan->ops->packet_size(config, buf, &ps); + ret = ops->packet_size(config, buf, &ps); if (ret < 0) goto error; return put_u64(ps, arg); @@ -1513,9 +1489,7 @@ static long lttng_stream_ring_buffer_compat_ioctl(struct file *filp, { uint64_t si; - if (!lttng_chan->ops) - goto error; - ret = lttng_chan->ops->stream_id(config, buf, &si); + ret = ops->stream_id(config, buf, &si); if (ret < 0) goto error; return put_u64(si, arg); @@ -1524,9 +1498,7 @@ static long lttng_stream_ring_buffer_compat_ioctl(struct file *filp, { uint64_t ts; - if (!lttng_chan->ops) - goto error; - ret = lttng_chan->ops->current_timestamp(config, buf, &ts); + ret = ops->current_timestamp(config, buf, &ts); if (ret < 0) goto error; return put_u64(ts, arg); diff --git a/lttng-ring-buffer-client.h b/lttng-ring-buffer-client.h index c186c60b..288cc325 100644 --- a/lttng-ring-buffer-client.h +++ b/lttng-ring-buffer-client.h @@ -32,6 +32,8 @@ #define LTTNG_COMPACT_EVENT_BITS 5 #define LTTNG_COMPACT_TSC_BITS 27 +static struct lttng_transport lttng_relay_transport; + /* * Keep the natural field alignment for _each field_ within this structure if * you ever add/remove a field from this header. Packed attribute is not used @@ -486,6 +488,18 @@ static const struct lib_ring_buffer_config client_config = { .wakeup = RING_BUFFER_WAKEUP_BY_TIMER, }; +static +void release_priv_ops(void *priv_ops) +{ + module_put(THIS_MODULE); +} + +static +void lttng_channel_destroy(struct channel *chan) +{ + channel_destroy(chan); +} + static struct channel *_channel_create(const char *name, struct lttng_channel *lttng_chan, void *buf_addr, @@ -493,15 +507,28 @@ struct channel *_channel_create(const char *name, unsigned int switch_timer_interval, unsigned int read_timer_interval) { - return channel_create(&client_config, name, lttng_chan, buf_addr, + struct channel *chan; + + chan = channel_create(&client_config, name, lttng_chan, buf_addr, subbuf_size, num_subbuf, switch_timer_interval, read_timer_interval); -} + if (chan) { + /* + * Ensure this module is not unloaded before we finish + * using lttng_relay_transport.ops. + */ + if (!try_module_get(THIS_MODULE)) { + printk(KERN_WARNING "LTT : Can't lock transport module.\n"); + goto error; + } + chan->backend.priv_ops = <tng_relay_transport.ops; + chan->backend.release_priv_ops = release_priv_ops; + } + return chan; -static -void lttng_channel_destroy(struct channel *chan) -{ - channel_destroy(chan); +error: + lttng_channel_destroy(chan); + return NULL; } static diff --git a/lttng-ring-buffer-metadata-client.h b/lttng-ring-buffer-metadata-client.h index bb91f4d4..5c8a1df9 100644 --- a/lttng-ring-buffer-metadata-client.h +++ b/lttng-ring-buffer-metadata-client.h @@ -26,6 +26,8 @@ #include "lttng-events.h" #include "lttng-tracer.h" +static struct lttng_transport lttng_relay_transport; + struct metadata_packet_header { uint32_t magic; /* 0x75D11D57 */ uint8_t uuid[16]; /* Unique Universal Identifier */ @@ -216,6 +218,18 @@ static const struct lib_ring_buffer_config client_config = { .wakeup = RING_BUFFER_WAKEUP_BY_TIMER, }; +static +void release_priv_ops(void *priv_ops) +{ + module_put(THIS_MODULE); +} + +static +void lttng_channel_destroy(struct channel *chan) +{ + channel_destroy(chan); +} + static struct channel *_channel_create(const char *name, struct lttng_channel *lttng_chan, void *buf_addr, @@ -223,15 +237,28 @@ struct channel *_channel_create(const char *name, unsigned int switch_timer_interval, unsigned int read_timer_interval) { - return channel_create(&client_config, name, lttng_chan, buf_addr, + struct channel *chan; + + chan = channel_create(&client_config, name, lttng_chan, buf_addr, subbuf_size, num_subbuf, switch_timer_interval, read_timer_interval); -} + if (chan) { + /* + * Ensure this module is not unloaded before we finish + * using lttng_relay_transport.ops. + */ + if (!try_module_get(THIS_MODULE)) { + printk(KERN_WARNING "LTT : Can't lock transport module.\n"); + goto error; + } + chan->backend.priv_ops = <tng_relay_transport.ops; + chan->backend.release_priv_ops = release_priv_ops; + } + return chan; -static -void lttng_channel_destroy(struct channel *chan) -{ - channel_destroy(chan); +error: + lttng_channel_destroy(chan); + return NULL; } static -- 2.34.1