Fix: truncation of text array and sequences by NULL terminator

The recent refactor does not take into account that text array and text
sequence types behave like Pascal Strings and not C strings, which means
the NULL terminator is optional, and the string size is delimited by the
array or sequence length.

Introduce a new lib_ring_buffer_pstrcpy() to handle copying into a ring
buffer Pascal String, and use it for array and sequence of text.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Fixes: 2792781482a5 ("ABI refactoring: sequence and array of text: copy input as string")
Fixes: #1301
Change-Id: Idcc13f061d5229496476f42dce84c2f395b7f6e6

diff --git a/include/lttng/ust-events.h b/include/lttng/ust-events.h
index ca3d19818b2bc6dd19134d950afc889dfa52395f..a418bb9811469ddc499bbddecd83dec5ccb828d3 100644 (file)
--- a/include/lttng/ust-events.h
+++ b/include/lttng/ust-events.h
@@ -451,7 +451,7 @@ struct lttng_ust_channel_buffer_ops {
                        const void *src, size_t len);
        void (*event_strcpy)(struct lttng_ust_lib_ring_buffer_ctx *ctx,
                        const char *src, size_t len);
-       void (*event_strcpy_pad)(struct lttng_ust_lib_ring_buffer_ctx *ctx,
+       void (*event_pstrcpy_pad)(struct lttng_ust_lib_ring_buffer_ctx *ctx,
                        const char *src, size_t len);
 
        /* End of base ABI. Fields below should be used after checking struct_size. */

diff --git a/include/lttng/ust-tracepoint-event.h b/include/lttng/ust-tracepoint-event.h
index 22c97438ba589c3c66694abbe69b5be98f511fb8..d12afa0b84ea35f936407ac380eab216fdee3b6d 100644 (file)
--- a/include/lttng/ust-tracepoint-event.h
+++ b/include/lttng/ust-tracepoint-event.h
@@ -707,7 +707,7 @@ size_t __event_get_align__##_provider##___##_name(_TP_ARGS_PROTO(_args))      \
        if (lttng_ust_string_encoding_##_encoding == lttng_ust_string_encoding_none) \
                __chan->ops->event_write(&__ctx, _src, sizeof(_type) * (_length)); \
        else                                                            \
-               __chan->ops->event_strcpy_pad(&__ctx, (const char *) (_src), _length); \
+               __chan->ops->event_pstrcpy_pad(&__ctx, (const char *) (_src), _length); \
 
 #undef _ctf_sequence_encoded
 #define _ctf_sequence_encoded(_type, _item, _src, _byte_order, _length_type, \
@@ -722,7 +722,7 @@ size_t __event_get_align__##_provider##___##_name(_TP_ARGS_PROTO(_args))      \
                __chan->ops->event_write(&__ctx, _src,                  \
                        sizeof(_type) * __get_dynamic_len(dest));       \
        else                                                            \
-               __chan->ops->event_strcpy_pad(&__ctx, (const char *) (_src), __get_dynamic_len(dest)); \
+               __chan->ops->event_pstrcpy_pad(&__ctx, (const char *) (_src), __get_dynamic_len(dest)); \
 
 #undef _ctf_string
 #define _ctf_string(_item, _src, _nowrite)                             \

diff --git a/liblttng-ust/lttng-ring-buffer-client.h b/liblttng-ust/lttng-ring-buffer-client.h
index 84457f0a04d97ee5c1e48873b3a1a07b45cc358c..8e14acf5fc3caebad3ad2ad54c0ed52001a979c2 100644 (file)
--- a/liblttng-ust/lttng-ring-buffer-client.h
+++ b/liblttng-ust/lttng-ring-buffer-client.h
@@ -764,10 +764,10 @@ void lttng_event_strcpy(struct lttng_ust_lib_ring_buffer_ctx *ctx, const char *s
 }
 
 static
-void lttng_event_strcpy_pad(struct lttng_ust_lib_ring_buffer_ctx *ctx,
+void lttng_event_pstrcpy_pad(struct lttng_ust_lib_ring_buffer_ctx *ctx,
                     const char *src, size_t len)
 {
-       lib_ring_buffer_strcpy(&client_config, ctx, src, len, '\0');
+       lib_ring_buffer_pstrcpy(&client_config, ctx, src, len, '\0');
 }
 
 #if 0
@@ -833,7 +833,7 @@ static struct lttng_transport lttng_relay_transport = {
                .event_commit = lttng_event_commit,
                .event_write = lttng_event_write,
                .event_strcpy = lttng_event_strcpy,
-               .event_strcpy_pad = lttng_event_strcpy_pad,
+               .event_pstrcpy_pad = lttng_event_pstrcpy_pad,
        },
        .client_config = &client_config,
 };

diff --git a/libringbuffer/backend.h b/libringbuffer/backend.h
index bced86d96885950e4842a7c394508e86960a978c..698994c21881a78a7c7beb160c0daa3a47e689fe 100644 (file)
--- a/libringbuffer/backend.h
+++ b/libringbuffer/backend.h
@@ -182,6 +182,64 @@ void lib_ring_buffer_strcpy(const struct lttng_ust_lib_ring_buffer_config *confi
        ctx->buf_offset += len;
 }
 
+/**
+ * lib_ring_buffer_pstrcpy - write to a buffer backend P-string
+ * @config : ring buffer instance configuration
+ * @ctx: ring buffer context. (input arguments only)
+ * @src : source pointer to copy from
+ * @len : length of data to copy
+ * @pad : character to use for padding
+ *
+ * This function copies up to @len bytes of data from a source pointer
+ * to a Pascal String into the buffer backend. If a terminating '\0'
+ * character is found in @src before @len characters are copied, pad the
+ * buffer with @pad characters (e.g.  '\0').
+ *
+ * The length of the pascal strings in the ring buffer is explicit: it
+ * is either the array or sequence length.
+ */
+static inline __attribute__((always_inline))
+void lib_ring_buffer_pstrcpy(const struct lttng_ust_lib_ring_buffer_config *config,
+                          struct lttng_ust_lib_ring_buffer_ctx *ctx,
+                          const char *src, size_t len, char pad)
+{
+       struct channel_backend *chanb = &ctx->chan->backend;
+       struct lttng_ust_shm_handle *handle = ctx->handle;
+       size_t count;
+       size_t offset = ctx->buf_offset;
+       struct lttng_ust_lib_ring_buffer_backend_pages *backend_pages;
+       void *p;
+
+       if (caa_unlikely(!len))
+               return;
+       /*
+        * Underlying layer should never ask for writes across
+        * subbuffers.
+        */
+       CHAN_WARN_ON(chanb, (offset & (chanb->buf_size - 1)) + len > chanb->buf_size);
+       backend_pages = lib_ring_buffer_get_backend_pages_from_ctx(config, ctx);
+       if (caa_unlikely(!backend_pages)) {
+               if (lib_ring_buffer_backend_get_pages(config, ctx, &backend_pages))
+                       return;
+       }
+       p = shmp_index(handle, backend_pages->p, offset & (chanb->subbuf_size - 1));
+       if (caa_unlikely(!p))
+               return;
+
+       count = lib_ring_buffer_do_strcpy(config, p, src, len);
+       offset += count;
+       /* Padding */
+       if (caa_unlikely(count < len)) {
+               size_t pad_len = len - count;
+
+               p = shmp_index(handle, backend_pages->p, offset & (chanb->subbuf_size - 1));
+               if (caa_unlikely(!p))
+                       return;
+               lib_ring_buffer_do_memset(p, pad, pad_len);
+       }
+       ctx->buf_offset += len;
+}
+
 /*
  * This accessor counts the number of unread records in a buffer.
  * It only provides a consistent value if no reads not writes are performed