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 <mathieu.desnoyers@efficios.com>

diff --git a/lib/ringbuffer/backend_types.h b/lib/ringbuffer/backend_types.h
index 6813dd8b47a4127c3dbacc717c50742f96bbe561..1577c812bb93c1df71cb5f736d8c1de6c8a933f6 100644 (file)
--- 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 9bb4184143902472d35e474563a2a84f86ac31a8..fc8d54139c446e0906b9b0560af3a2a568fa7992 100644 (file)
--- 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 1cc9510110dbc216ecf02c4ca8157dfc8f12a78e..261a0ad3f8a1bf0ae4af6e76c9a2a246115589b4 100644 (file)
--- 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 c186c60b4a3eb62f68fef575cd2caf07860eeca0..288cc32509dcde332f3aa3d9eab7b2a39b9b2f32 100644 (file)
--- 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 = &lttng_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 bb91f4d4aaa4a049af5adf40a483f43afefb15bb..5c8a1df9e1828e76f432c211697d36024b523d80 100644 (file)
--- 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 = &lttng_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