[PATCH lttng-tools 1/3] Fix: Filter bytecode alloc buffer size must be a power of 2

[PATCH lttng-tools 1/3] Fix: Filter bytecode alloc buffer size must be a power of 2

[Christian Babeux]

The current allocation policy for the filter bytecode buffer is to double
the size each time the underlying buffer can no longer contain the entire
bytecode plus padding.

In some cases, the initial allocation length is not a multiple of 2, thus
possibly leading to odd-looking allocation size each time the buffer size
is doubled.

Signed-off-by: Christian Babeux <christian.babeux@efficios.com>
---
 .../filter/filter-visitor-generate-bytecode.c      | 41 +++++++++++++++++++++-
 1 file changed, 40 insertions(+), 1 deletion(-)

diff --git a/src/lib/lttng-ctl/filter/filter-visitor-generate-bytecode.c b/src/lib/lttng-ctl/filter/filter-visitor-generate-bytecode.c
index 36d35c5..71da21c 100644
--- a/src/lib/lttng-ctl/filter/filter-visitor-generate-bytecode.c
+++ b/src/lib/lttng-ctl/filter/filter-visitor-generate-bytecode.c
@@ -38,6 +38,45 @@ static
 int recursive_visit_gen_bytecode(struct filter_parser_ctx *ctx,
 		struct ir_op *node);
 
+static inline int fls(unsigned int x)
+{
+	int r = 32;
+
+	if (!x)
+		return 0;
+	if (!(x & 0xFFFF0000U)) {
+		x <<= 16;
+		r -= 16;
+	}
+	if (!(x & 0xFF000000U)) {
+		x <<= 8;
+		r -= 8;
+	}
+	if (!(x & 0xF0000000U)) {
+		x <<= 4;
+		r -= 4;
+	}
+	if (!(x & 0xC0000000U)) {
+		x <<= 2;
+		r -= 2;
+	}
+	if (!(x & 0x80000000U)) {
+		x <<= 1;
+		r -= 1;
+	}
+	return r;
+}
+
+static inline int get_count_order(unsigned int count)
+{
+	int order;
+
+	order = fls(count) - 1;
+	if (count & (count - 1))
+		order++;
+	return order;
+}
+
 static
 int bytecode_init(struct lttng_filter_bytecode_alloc **fb)
 {
@@ -58,7 +97,7 @@ int32_t bytecode_reserve(struct lttng_filter_bytecode_alloc **fb, uint32_t align
 
 	if ((*fb)->b.len + padding + len > (*fb)->alloc_len) {
 		uint32_t new_len =
-			max_t(uint32_t, (*fb)->b.len + padding + len,
+			max_t(uint32_t, 1U << get_count_order((*fb)->b.len + padding + len),
 				(*fb)->alloc_len << 1);
 		uint32_t old_len = (*fb)->alloc_len;
 
-- 
1.7.11.4

[PATCH lttng-tools 2/3] Fix: Typo in LTTNG_FILTER_MAX_LEN

[Christian Babeux]

Signed-off-by: Christian Babeux <christian.babeux@efficios.com>
---
 src/bin/lttng-sessiond/main.c            | 2 +-
 src/common/sessiond-comm/sessiond-comm.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/bin/lttng-sessiond/main.c b/src/bin/lttng-sessiond/main.c
index 318da74..730ac65 100644
--- a/src/bin/lttng-sessiond/main.c
+++ b/src/bin/lttng-sessiond/main.c
@@ -2659,7 +2659,7 @@ skip_domain:
 	{
 		struct lttng_filter_bytecode *bytecode;
 
-		if (cmd_ctx->lsm->u.filter.bytecode_len > 65336) {
+		if (cmd_ctx->lsm->u.filter.bytecode_len > LTTNG_FILTER_MAX_LEN) {
 			ret = LTTNG_ERR_FILTER_INVAL;
 			goto error;
 		}
diff --git a/src/common/sessiond-comm/sessiond-comm.h b/src/common/sessiond-comm/sessiond-comm.h
index 32ce384..ff22875 100644
--- a/src/common/sessiond-comm/sessiond-comm.h
+++ b/src/common/sessiond-comm/sessiond-comm.h
@@ -208,7 +208,7 @@ struct lttcomm_session_msg {
 	} u;
 };
 
-#define LTTNG_FILTER_MAX_LEN	65336
+#define LTTNG_FILTER_MAX_LEN	65535
 
 /*
  * Filter bytecode data. The reloc table is located at the end of the
-- 
1.7.11.4

[PATCH lttng-tools 3/3] Fix: Generation of bytecode longer than 32768 bytes fails

[Christian Babeux]

The bytecode buffer length field is currently limited to a uint16_t.
A larger buffer, lttng_filter_bytecode_alloc, is the underlying storage
for the bytecode.

The current allocation policy dictate that the alloc buffer size must be
doubled everytime the bytecode size plus padding exceeds its capacity.

A problem arise when generating bytecode larger than 32768 bytes.

e.g.:

Legend
* required_len: new bytecode len
* old_len: current alloc_len
* new_len: new alloc_len

src/bin/lttng/lttng enable-event event:bla -s foo -u --filter "`perl -e 'print "intfield" . " && 1" x2730'`"
UST event ust_tests_hello:tptest created in channel channel0
[debug liblttng-ctl] Generating IR... [debug liblttng-ctl] done
[debug liblttng-ctl] Validating IR... [debug liblttng-ctl] done
[debug liblttng-ctl] Generating bytecode... required_len = 11, old_len = 4, new_len = 16
required_len = 7, old_len = 4, new_len = 8
required_len = 16, old_len = 8, new_len = 16
required_len = 19, old_len = 16, new_len = 32
required_len = 40, old_len = 32, new_len = 64
required_len = 67, old_len = 64, new_len = 128
required_len = 136, old_len = 128, new_len = 256
required_len = 259, old_len = 256, new_len = 512
required_len = 520, old_len = 512, new_len = 1024
required_len = 1027, old_len = 1024, new_len = 2048
required_len = 2056, old_len = 2048, new_len = 4096
required_len = 4099, old_len = 4096, new_len = 8192
required_len = 8200, old_len = 8192, new_len = 16384
required_len = 16387, old_len = 16384, new_len = 32768
required_len = 32776, old_len = 32768, new_len = 65536 <-- Overflow 16-bits
Generate bytecode error
Error: Error setting filter

The last new_len exceed the range of 16-bits values. In order to support
the largest bytecode length (65535), the underlying alloc buffer len must
be able to store more than 65535. Fix this by using a uint32_t for alloc_len.

Also, add a check to ensure that a bytecode longer than LTTNG_FILTER_MAX_LEN
(65535) bytes can't be generated.

Signed-off-by: Christian Babeux <christian.babeux@efficios.com>
---
 src/lib/lttng-ctl/filter/filter-bytecode.h                  | 2 +-
 src/lib/lttng-ctl/filter/filter-visitor-generate-bytecode.c | 5 +++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/lib/lttng-ctl/filter/filter-bytecode.h b/src/lib/lttng-ctl/filter/filter-bytecode.h
index 5d2559d..d364ee2 100644
--- a/src/lib/lttng-ctl/filter/filter-bytecode.h
+++ b/src/lib/lttng-ctl/filter/filter-bytecode.h
@@ -176,7 +176,7 @@ struct return_op {
 } __attribute__((packed));
 
 struct lttng_filter_bytecode_alloc {
-	uint16_t alloc_len;
+	uint32_t alloc_len;
 	struct lttng_filter_bytecode b;
 };
 
diff --git a/src/lib/lttng-ctl/filter/filter-visitor-generate-bytecode.c b/src/lib/lttng-ctl/filter/filter-visitor-generate-bytecode.c
index 71da21c..98f8375 100644
--- a/src/lib/lttng-ctl/filter/filter-visitor-generate-bytecode.c
+++ b/src/lib/lttng-ctl/filter/filter-visitor-generate-bytecode.c
@@ -95,14 +95,15 @@ int32_t bytecode_reserve(struct lttng_filter_bytecode_alloc **fb, uint32_t align
 	int32_t ret;
 	uint32_t padding = offset_align((*fb)->b.len, align);
 
+	if ((*fb)->b.len + padding + len > LTTNG_FILTER_MAX_LEN)
+		return -EINVAL;
+
 	if ((*fb)->b.len + padding + len > (*fb)->alloc_len) {
 		uint32_t new_len =
 			max_t(uint32_t, 1U << get_count_order((*fb)->b.len + padding + len),
 				(*fb)->alloc_len << 1);
 		uint32_t old_len = (*fb)->alloc_len;
 
-		if (new_len > 0xFFFF)
-			return -EINVAL;
 		*fb = realloc(*fb, sizeof(struct lttng_filter_bytecode_alloc) + new_len);
 		if (!*fb)
 			return -ENOMEM;
-- 
1.7.11.4

Re: [PATCH lttng-tools 2/3] Fix: Typo in LTTNG_FILTER_MAX_LEN

[Mathieu Desnoyers]

* Christian Babeux (christian.babeux@efficios.com) wrote:
> 
> Signed-off-by: Christian Babeux <christian.babeux@efficios.com>
> ---
>  src/bin/lttng-sessiond/main.c            | 2 +-
>  src/common/sessiond-comm/sessiond-comm.h | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/bin/lttng-sessiond/main.c b/src/bin/lttng-sessiond/main.c
> index 318da74..730ac65 100644
> --- a/src/bin/lttng-sessiond/main.c
> +++ b/src/bin/lttng-sessiond/main.c
> @@ -2659,7 +2659,7 @@ skip_domain:
>  	{
>  		struct lttng_filter_bytecode *bytecode;
>  
> -		if (cmd_ctx->lsm->u.filter.bytecode_len > 65336) {
> +		if (cmd_ctx->lsm->u.filter.bytecode_len > LTTNG_FILTER_MAX_LEN) {
>  			ret = LTTNG_ERR_FILTER_INVAL;
>  			goto error;
>  		}
> diff --git a/src/common/sessiond-comm/sessiond-comm.h b/src/common/sessiond-comm/sessiond-comm.h
> index 32ce384..ff22875 100644
> --- a/src/common/sessiond-comm/sessiond-comm.h
> +++ b/src/common/sessiond-comm/sessiond-comm.h
> @@ -208,7 +208,7 @@ struct lttcomm_session_msg {
>  	} u;
>  };
>  
> -#define LTTNG_FILTER_MAX_LEN	65336
> +#define LTTNG_FILTER_MAX_LEN	65535

This should be 65536.

Thanks,

Mathieu

>  
>  /*
>   * Filter bytecode data. The reloc table is located at the end of the
> -- 
> 1.7.11.4
> 

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

Re: [PATCH lttng-tools 1/3] Fix: Filter bytecode alloc buffer size must be a power of 2

[Mathieu Desnoyers]

* Christian Babeux (christian.babeux@efficios.com) wrote:
> The current allocation policy for the filter bytecode buffer is to double
> the size each time the underlying buffer can no longer contain the entire
> bytecode plus padding.
> 
> In some cases, the initial allocation length is not a multiple of 2, thus
> possibly leading to odd-looking allocation size each time the buffer size
> is doubled.
> 
> Signed-off-by: Christian Babeux <christian.babeux@efficios.com>

Acked-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>

> ---
>  .../filter/filter-visitor-generate-bytecode.c      | 41 +++++++++++++++++++++-
>  1 file changed, 40 insertions(+), 1 deletion(-)
> 
> diff --git a/src/lib/lttng-ctl/filter/filter-visitor-generate-bytecode.c b/src/lib/lttng-ctl/filter/filter-visitor-generate-bytecode.c
> index 36d35c5..71da21c 100644
> --- a/src/lib/lttng-ctl/filter/filter-visitor-generate-bytecode.c
> +++ b/src/lib/lttng-ctl/filter/filter-visitor-generate-bytecode.c
> @@ -38,6 +38,45 @@ static
>  int recursive_visit_gen_bytecode(struct filter_parser_ctx *ctx,
>  		struct ir_op *node);
>  
> +static inline int fls(unsigned int x)
> +{
> +	int r = 32;
> +
> +	if (!x)
> +		return 0;
> +	if (!(x & 0xFFFF0000U)) {
> +		x <<= 16;
> +		r -= 16;
> +	}
> +	if (!(x & 0xFF000000U)) {
> +		x <<= 8;
> +		r -= 8;
> +	}
> +	if (!(x & 0xF0000000U)) {
> +		x <<= 4;
> +		r -= 4;
> +	}
> +	if (!(x & 0xC0000000U)) {
> +		x <<= 2;
> +		r -= 2;
> +	}
> +	if (!(x & 0x80000000U)) {
> +		x <<= 1;
> +		r -= 1;
> +	}
> +	return r;
> +}
> +
> +static inline int get_count_order(unsigned int count)
> +{
> +	int order;
> +
> +	order = fls(count) - 1;
> +	if (count & (count - 1))
> +		order++;
> +	return order;
> +}
> +
>  static
>  int bytecode_init(struct lttng_filter_bytecode_alloc **fb)
>  {
> @@ -58,7 +97,7 @@ int32_t bytecode_reserve(struct lttng_filter_bytecode_alloc **fb, uint32_t align
>  
>  	if ((*fb)->b.len + padding + len > (*fb)->alloc_len) {
>  		uint32_t new_len =
> -			max_t(uint32_t, (*fb)->b.len + padding + len,
> +			max_t(uint32_t, 1U << get_count_order((*fb)->b.len + padding + len),
>  				(*fb)->alloc_len << 1);
>  		uint32_t old_len = (*fb)->alloc_len;
>  
> -- 
> 1.7.11.4
> 

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

Re: [PATCH lttng-tools 3/3] Fix: Generation of bytecode longer than 32768 bytes fails

[Mathieu Desnoyers]

* Christian Babeux (christian.babeux@efficios.com) wrote:
> The bytecode buffer length field is currently limited to a uint16_t.
> A larger buffer, lttng_filter_bytecode_alloc, is the underlying storage
> for the bytecode.
> 
> The current allocation policy dictate that the alloc buffer size must be
> doubled everytime the bytecode size plus padding exceeds its capacity.
> 
> A problem arise when generating bytecode larger than 32768 bytes.
> 
> e.g.:
> 
> Legend
> * required_len: new bytecode len
> * old_len: current alloc_len
> * new_len: new alloc_len
> 
> src/bin/lttng/lttng enable-event event:bla -s foo -u --filter "`perl -e 'print "intfield" . " && 1" x2730'`"
> UST event ust_tests_hello:tptest created in channel channel0
> [debug liblttng-ctl] Generating IR... [debug liblttng-ctl] done
> [debug liblttng-ctl] Validating IR... [debug liblttng-ctl] done
> [debug liblttng-ctl] Generating bytecode... required_len = 11, old_len = 4, new_len = 16
> required_len = 7, old_len = 4, new_len = 8
> required_len = 16, old_len = 8, new_len = 16
> required_len = 19, old_len = 16, new_len = 32
> required_len = 40, old_len = 32, new_len = 64
> required_len = 67, old_len = 64, new_len = 128
> required_len = 136, old_len = 128, new_len = 256
> required_len = 259, old_len = 256, new_len = 512
> required_len = 520, old_len = 512, new_len = 1024
> required_len = 1027, old_len = 1024, new_len = 2048
> required_len = 2056, old_len = 2048, new_len = 4096
> required_len = 4099, old_len = 4096, new_len = 8192
> required_len = 8200, old_len = 8192, new_len = 16384
> required_len = 16387, old_len = 16384, new_len = 32768
> required_len = 32776, old_len = 32768, new_len = 65536 <-- Overflow 16-bits
> Generate bytecode error
> Error: Error setting filter
> 
> The last new_len exceed the range of 16-bits values. In order to support
> the largest bytecode length (65535), the underlying alloc buffer len must
> be able to store more than 65535. Fix this by using a uint32_t for alloc_len.
> 
> Also, add a check to ensure that a bytecode longer than LTTNG_FILTER_MAX_LEN
> (65535) bytes can't be generated.
> 
> Signed-off-by: Christian Babeux <christian.babeux@efficios.com>

Acked-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>

> ---
>  src/lib/lttng-ctl/filter/filter-bytecode.h                  | 2 +-
>  src/lib/lttng-ctl/filter/filter-visitor-generate-bytecode.c | 5 +++--
>  2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/src/lib/lttng-ctl/filter/filter-bytecode.h b/src/lib/lttng-ctl/filter/filter-bytecode.h
> index 5d2559d..d364ee2 100644
> --- a/src/lib/lttng-ctl/filter/filter-bytecode.h
> +++ b/src/lib/lttng-ctl/filter/filter-bytecode.h
> @@ -176,7 +176,7 @@ struct return_op {
>  } __attribute__((packed));
>  
>  struct lttng_filter_bytecode_alloc {
> -	uint16_t alloc_len;
> +	uint32_t alloc_len;
>  	struct lttng_filter_bytecode b;
>  };
>  
> diff --git a/src/lib/lttng-ctl/filter/filter-visitor-generate-bytecode.c b/src/lib/lttng-ctl/filter/filter-visitor-generate-bytecode.c
> index 71da21c..98f8375 100644
> --- a/src/lib/lttng-ctl/filter/filter-visitor-generate-bytecode.c
> +++ b/src/lib/lttng-ctl/filter/filter-visitor-generate-bytecode.c
> @@ -95,14 +95,15 @@ int32_t bytecode_reserve(struct lttng_filter_bytecode_alloc **fb, uint32_t align
>  	int32_t ret;
>  	uint32_t padding = offset_align((*fb)->b.len, align);
>  
> +	if ((*fb)->b.len + padding + len > LTTNG_FILTER_MAX_LEN)
> +		return -EINVAL;
> +
>  	if ((*fb)->b.len + padding + len > (*fb)->alloc_len) {
>  		uint32_t new_len =
>  			max_t(uint32_t, 1U << get_count_order((*fb)->b.len + padding + len),
>  				(*fb)->alloc_len << 1);
>  		uint32_t old_len = (*fb)->alloc_len;
>  
> -		if (new_len > 0xFFFF)
> -			return -EINVAL;
>  		*fb = realloc(*fb, sizeof(struct lttng_filter_bytecode_alloc) + new_len);
>  		if (!*fb)
>  			return -ENOMEM;
> -- 
> 1.7.11.4
> 

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

Re: [PATCH lttng-tools 0/3] Filter allocation and large bytecode fixes

[David Goulet]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

All merged except 2/3.

David

Christian Babeux:
> Hi all,
> 
> While testing the new filtering feature, some issues were
> discovered when generating large bytecode (more than 32768 bytes).
> 
> The first patch fix an issue with the buffer size not being a power
> of 2, thus leading to odd-looking alloc buffer size.
> 
> The second patch is a typo fix.
> 
> The third patch fix an issue when generating a bytecode longer than
> 32768 bytes. The alloc_len of the underlying bytecode buffer was
> artificially limiting the bytecode size to 32768 because its length
> was overflowing the range of 16-bits values.
> 
> Thanks,
> 
> Christian Babeux (3): Fix: Filter bytecode alloc buffer size must
> be a power of 2 Fix: Typo in LTTNG_FILTER_MAX_LEN Fix: Generation
> of bytecode longer than 32768 bytes fails
> 
> src/bin/lttng-sessiond/main.c                      |  2 +- 
> src/common/sessiond-comm/sessiond-comm.h           |  2 +- 
> src/lib/lttng-ctl/filter/filter-bytecode.h         |  2 +- 
> .../filter/filter-visitor-generate-bytecode.c      | 46
> ++++++++++++++++++++-- 4 files changed, 46 insertions(+), 6
> deletions(-)
> 
-----BEGIN PGP SIGNATURE-----

iQEcBAEBCgAGBQJQRiFJAAoJEELoaioR9I02CJkIAK17zl/81EYeECRffELtmNDa
KwSLfhPl2Xf0wVPm2TshxLK5Lc3yE/14KYrMbeTP4sYB87Vn66k4HqZIqWHtYtaS
TKI1qjruhuC4LqySHTUcuw4TWC0j96ZoM5iSc97lj9dqCmrRA8A9msnbfssPi/FQ
g6tIsScpozrzHbBNYL9aCBqcrdX60tMm2fZ7XqJZi5f3yy7QNlWKVNWrFWHHxMks
PATSwgQhhyVi8GsZj33n+bMBuiXbMxii1S5h7zVedxCXWdL5w1wtr9l6WmD/l8jw
HUtCS+BWh0Fldh1/M2ua2GyfvAGJuyxJyJPC/jPdYF9dJWf+/3AoWMn6poIbxPA=
=pRxc
-----END PGP SIGNATURE-----

[PATCH v2 lttng-tools] Filter allocation and large bytecode fixes

[Christian Babeux]

Hi all,

After some discussion with Mathieu, it was decided that we should
accept filter bytecode with a length of 65536 bytes. In order to fully support
this length, the len field of lttng_ust_filter_bytecode must be able to hold
that value, so we must change the len type from a uint16_t to a uint32_t.

Also, the relocation table offset field must be able to hold a value
larger than 65535 since the table is located at the end of the generated
bytecode. The field type must be changed from a uint16_t to a uint32_t.

Theses changes _will_ break the filter ABI. Since we are in RC and the
lttng_ust_filter_bytecode struct has only been introduced with the
new filtering feature this seems to be a reasonable approach. This
change must also be reflected on the lttng-ust side. A subsequent patch
for lttng-ust will address this particular concern.

Changelog v2:

1/3 - Already merged.
2/3 - Respin. Support 65536 bytes bytecode len + filter ABI changes.
3/3 - Already merged.

Thanks,

Christian Babeux (1):
  Fix: Accept bytecode of length 65536 bytes

 src/bin/lttng-sessiond/lttng-ust-abi.h                      | 6 +++---
 src/common/sessiond-comm/sessiond-comm.h                    | 6 +++---
 src/lib/lttng-ctl/filter/filter-visitor-generate-bytecode.c | 2 +-
 3 files changed, 7 insertions(+), 7 deletions(-)

-- 
1.7.11.4

[PATCH v2 lttng-tools] Fix: Accept bytecode of length 65536 bytes

[Christian Babeux]

In order to support the filter bytecode maximum length (65536 bytes),
the lttng_ust_filter_bytecode len field type must be able to
hold more than a uint16_t. Change the field type to a uint32_t.

Also, since the relocation table is located at the end of the actual
bytecode, the reloc_table_offset (reloc_offset in ust-abi) field must
support offset values larger than 65535. Change the field type to a
uint32_t. This change will allow support of relocation table appended
to larger bytecode without breaking the ABI if the need arise in the
future.

Both changes currently breaks the filter ABI, but this should be a
reasonable compromise since the filtering feature has not been
released yet.

Signed-off-by: Christian Babeux <christian.babeux@efficios.com>
---
 src/bin/lttng-sessiond/lttng-ust-abi.h                      | 6 +++---
 src/common/sessiond-comm/sessiond-comm.h                    | 6 +++---
 src/lib/lttng-ctl/filter/filter-visitor-generate-bytecode.c | 2 +-
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/bin/lttng-sessiond/lttng-ust-abi.h b/src/bin/lttng-sessiond/lttng-ust-abi.h
index d8b10c2..504c060 100644
--- a/src/bin/lttng-sessiond/lttng-ust-abi.h
+++ b/src/bin/lttng-sessiond/lttng-ust-abi.h
@@ -168,10 +168,10 @@ struct lttng_ust_calibrate {
 	} u;
 };
 
-#define FILTER_BYTECODE_MAX_LEN		65535
+#define FILTER_BYTECODE_MAX_LEN		65536
 struct lttng_ust_filter_bytecode {
-	uint16_t len;
-	uint16_t reloc_offset;
+	uint32_t len;
+	uint32_t reloc_offset;
 	char data[0];
 };
 
diff --git a/src/common/sessiond-comm/sessiond-comm.h b/src/common/sessiond-comm/sessiond-comm.h
index ff22875..62205f4 100644
--- a/src/common/sessiond-comm/sessiond-comm.h
+++ b/src/common/sessiond-comm/sessiond-comm.h
@@ -208,7 +208,7 @@ struct lttcomm_session_msg {
 	} u;
 };
 
-#define LTTNG_FILTER_MAX_LEN	65535
+#define LTTNG_FILTER_MAX_LEN	65536
 
 /*
  * Filter bytecode data. The reloc table is located at the end of the
@@ -216,8 +216,8 @@ struct lttcomm_session_msg {
  * starts at reloc_table_offset.
  */
 struct lttng_filter_bytecode {
-	uint16_t len;	/* len of data */
-	uint16_t reloc_table_offset;
+	uint32_t len;	/* len of data */
+	uint32_t reloc_table_offset;
 	char data[0];
 };
 
diff --git a/src/lib/lttng-ctl/filter/filter-visitor-generate-bytecode.c b/src/lib/lttng-ctl/filter/filter-visitor-generate-bytecode.c
index 98f8375..332a387 100644
--- a/src/lib/lttng-ctl/filter/filter-visitor-generate-bytecode.c
+++ b/src/lib/lttng-ctl/filter/filter-visitor-generate-bytecode.c
@@ -239,7 +239,7 @@ int visit_node_load(struct filter_parser_ctx *ctx, struct ir_op *node)
 		uint32_t insn_len = sizeof(struct load_op)
 			+ sizeof(struct field_ref);
 		struct field_ref ref_offset;
-		uint16_t reloc_offset;
+		uint32_t reloc_offset;
 
 		insn = calloc(insn_len, 1);
 		if (!insn)
-- 
1.7.11.4

Re: [PATCH v2 lttng-tools] Fix: Accept bytecode of length 65536 bytes

[Mathieu Desnoyers]

* Christian Babeux (christian.babeux@efficios.com) wrote:
> In order to support the filter bytecode maximum length (65536 bytes),
> the lttng_ust_filter_bytecode len field type must be able to
> hold more than a uint16_t. Change the field type to a uint32_t.
> 
> Also, since the relocation table is located at the end of the actual
> bytecode, the reloc_table_offset (reloc_offset in ust-abi) field must
> support offset values larger than 65535. Change the field type to a
> uint32_t. This change will allow support of relocation table appended
> to larger bytecode without breaking the ABI if the need arise in the
> future.
> 
> Both changes currently breaks the filter ABI, but this should be a
> reasonable compromise since the filtering feature has not been
> released yet.
> 
> Signed-off-by: Christian Babeux <christian.babeux@efficios.com>
> ---
>  src/bin/lttng-sessiond/lttng-ust-abi.h                      | 6 +++---
>  src/common/sessiond-comm/sessiond-comm.h                    | 6 +++---
>  src/lib/lttng-ctl/filter/filter-visitor-generate-bytecode.c | 2 +-
>  3 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/src/bin/lttng-sessiond/lttng-ust-abi.h b/src/bin/lttng-sessiond/lttng-ust-abi.h
> index d8b10c2..504c060 100644
> --- a/src/bin/lttng-sessiond/lttng-ust-abi.h
> +++ b/src/bin/lttng-sessiond/lttng-ust-abi.h
> @@ -168,10 +168,10 @@ struct lttng_ust_calibrate {
>  	} u;
>  };
>  
> -#define FILTER_BYTECODE_MAX_LEN		65535
> +#define FILTER_BYTECODE_MAX_LEN		65536
>  struct lttng_ust_filter_bytecode {
> -	uint16_t len;
> -	uint16_t reloc_offset;
> +	uint32_t len;
> +	uint32_t reloc_offset;
>  	char data[0];
>  };
>  
> diff --git a/src/common/sessiond-comm/sessiond-comm.h b/src/common/sessiond-comm/sessiond-comm.h
> index ff22875..62205f4 100644
> --- a/src/common/sessiond-comm/sessiond-comm.h
> +++ b/src/common/sessiond-comm/sessiond-comm.h
> @@ -208,7 +208,7 @@ struct lttcomm_session_msg {
>  	} u;
>  };
>  
> -#define LTTNG_FILTER_MAX_LEN	65535
> +#define LTTNG_FILTER_MAX_LEN	65536
>  
>  /*
>   * Filter bytecode data. The reloc table is located at the end of the
> @@ -216,8 +216,8 @@ struct lttcomm_session_msg {
>   * starts at reloc_table_offset.
>   */
>  struct lttng_filter_bytecode {
> -	uint16_t len;	/* len of data */
> -	uint16_t reloc_table_offset;
> +	uint32_t len;	/* len of data */
> +	uint32_t reloc_table_offset;

So you might want to add, at:

src/bin/lttng-sessiond/main.c, around line 3965 (under case
LTTNG_SET_FILTER):

                if (cmd_ctx->lsm->u.filter.reloc_table_offset
                                > LTTNG_FILTER_MAX_LEN - 1) {
                        ret = LTTNG_ERR_FILTER_INVAL;
                        goto error;
                }

and change:

                if (cmd_ctx->lsm->u.filter.bytecode_len > 65336) {

for

                if (cmd_ctx->lsm->u.filter.bytecode_len > LTTNG_FILTER_MAX_LEN) {

Thanks,

Mathieu


>  	char data[0];
>  };
>  
> diff --git a/src/lib/lttng-ctl/filter/filter-visitor-generate-bytecode.c b/src/lib/lttng-ctl/filter/filter-visitor-generate-bytecode.c
> index 98f8375..332a387 100644
> --- a/src/lib/lttng-ctl/filter/filter-visitor-generate-bytecode.c
> +++ b/src/lib/lttng-ctl/filter/filter-visitor-generate-bytecode.c
> @@ -239,7 +239,7 @@ int visit_node_load(struct filter_parser_ctx *ctx, struct ir_op *node)
>  		uint32_t insn_len = sizeof(struct load_op)
>  			+ sizeof(struct field_ref);
>  		struct field_ref ref_offset;
> -		uint16_t reloc_offset;
> +		uint32_t reloc_offset;
>  
>  		insn = calloc(insn_len, 1);
>  		if (!insn)
> -- 
> 1.7.11.4
> 

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

[PATCH v3 lttng-tools] Filter allocation and large bytecode fixes

[Christian Babeux]

Previous iteration of patch 2/3 was missing a typo fix in the
LTTNG_FILTER_MAX_LEN define. 

Also, Mathieu suggested that we put a check on the reloc_table_offset
in the sessiond to check its bounds when receiving the LTTNG_SET_FILTER
command. This is not necessary because the reloc_table_offset does not
exist in the filter struct of the lttcomm_session_msg union.

Changelog v3:

1/3 - Already merged.
2/3 - Add missing typo fix in LTTNG_FILTER_MAX_LEN define.
3/3 - Already merged.

Changelog v2:

1/3 - Already merged.
2/3 - Respin. Support 65536 bytes bytecode len + filter ABI changes.
3/3 - Already merged.

Thanks,

Christian Babeux (1):
  Fix: Accept bytecode of length 65536 bytes

 src/bin/lttng-sessiond/lttng-ust-abi.h                      | 6 +++---
 src/bin/lttng-sessiond/main.c                               | 2 +-
 src/common/sessiond-comm/sessiond-comm.h                    | 6 +++---
 src/lib/lttng-ctl/filter/filter-visitor-generate-bytecode.c | 2 +-
 4 files changed, 8 insertions(+), 8 deletions(-)

-- 
1.7.11.4

[PATCH v3 lttng-tools] Fix: Accept bytecode of length 65536 bytes

[Christian Babeux]

In order to support the filter bytecode maximum length (65536 bytes),
the lttng_ust_filter_bytecode len field type must be able to
hold more than a uint16_t. Change the field type to a uint32_t.

Also, since the relocation table is located at the end of the actual
bytecode, the reloc_table_offset (reloc_offset in ust-abi) field must
support offset values larger than 65535. Change the field type to a
uint32_t. This change will allow support of relocation table appended
to larger bytecode without breaking the ABI if the need arise in the
future.

Both changes currently breaks the filter ABI, but this should be a
reasonable compromise since the filtering feature has not been
released yet.

Signed-off-by: Christian Babeux <christian.babeux@efficios.com>
---
 src/bin/lttng-sessiond/lttng-ust-abi.h                      | 6 +++---
 src/bin/lttng-sessiond/main.c                               | 2 +-
 src/common/sessiond-comm/sessiond-comm.h                    | 6 +++---
 src/lib/lttng-ctl/filter/filter-visitor-generate-bytecode.c | 2 +-
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/bin/lttng-sessiond/lttng-ust-abi.h b/src/bin/lttng-sessiond/lttng-ust-abi.h
index d8b10c2..504c060 100644
--- a/src/bin/lttng-sessiond/lttng-ust-abi.h
+++ b/src/bin/lttng-sessiond/lttng-ust-abi.h
@@ -168,10 +168,10 @@ struct lttng_ust_calibrate {
 	} u;
 };
 
-#define FILTER_BYTECODE_MAX_LEN		65535
+#define FILTER_BYTECODE_MAX_LEN		65536
 struct lttng_ust_filter_bytecode {
-	uint16_t len;
-	uint16_t reloc_offset;
+	uint32_t len;
+	uint32_t reloc_offset;
 	char data[0];
 };
 
diff --git a/src/bin/lttng-sessiond/main.c b/src/bin/lttng-sessiond/main.c
index 318da74..730ac65 100644
--- a/src/bin/lttng-sessiond/main.c
+++ b/src/bin/lttng-sessiond/main.c
@@ -2659,7 +2659,7 @@ skip_domain:
 	{
 		struct lttng_filter_bytecode *bytecode;
 
-		if (cmd_ctx->lsm->u.filter.bytecode_len > 65336) {
+		if (cmd_ctx->lsm->u.filter.bytecode_len > LTTNG_FILTER_MAX_LEN) {
 			ret = LTTNG_ERR_FILTER_INVAL;
 			goto error;
 		}
diff --git a/src/common/sessiond-comm/sessiond-comm.h b/src/common/sessiond-comm/sessiond-comm.h
index 32ce384..62205f4 100644
--- a/src/common/sessiond-comm/sessiond-comm.h
+++ b/src/common/sessiond-comm/sessiond-comm.h
@@ -208,7 +208,7 @@ struct lttcomm_session_msg {
 	} u;
 };
 
-#define LTTNG_FILTER_MAX_LEN	65336
+#define LTTNG_FILTER_MAX_LEN	65536
 
 /*
  * Filter bytecode data. The reloc table is located at the end of the
@@ -216,8 +216,8 @@ struct lttcomm_session_msg {
  * starts at reloc_table_offset.
  */
 struct lttng_filter_bytecode {
-	uint16_t len;	/* len of data */
-	uint16_t reloc_table_offset;
+	uint32_t len;	/* len of data */
+	uint32_t reloc_table_offset;
 	char data[0];
 };
 
diff --git a/src/lib/lttng-ctl/filter/filter-visitor-generate-bytecode.c b/src/lib/lttng-ctl/filter/filter-visitor-generate-bytecode.c
index 98f8375..332a387 100644
--- a/src/lib/lttng-ctl/filter/filter-visitor-generate-bytecode.c
+++ b/src/lib/lttng-ctl/filter/filter-visitor-generate-bytecode.c
@@ -239,7 +239,7 @@ int visit_node_load(struct filter_parser_ctx *ctx, struct ir_op *node)
 		uint32_t insn_len = sizeof(struct load_op)
 			+ sizeof(struct field_ref);
 		struct field_ref ref_offset;
-		uint16_t reloc_offset;
+		uint32_t reloc_offset;
 
 		insn = calloc(insn_len, 1);
 		if (!insn)
-- 
1.7.11.4

Re: [PATCH v3 lttng-tools] Fix: Accept bytecode of length 65536 bytes

[Mathieu Desnoyers]

* Christian Babeux (christian.babeux@efficios.com) wrote:
> In order to support the filter bytecode maximum length (65536 bytes),
> the lttng_ust_filter_bytecode len field type must be able to
> hold more than a uint16_t. Change the field type to a uint32_t.
> 
> Also, since the relocation table is located at the end of the actual
> bytecode, the reloc_table_offset (reloc_offset in ust-abi) field must
> support offset values larger than 65535. Change the field type to a
> uint32_t. This change will allow support of relocation table appended
> to larger bytecode without breaking the ABI if the need arise in the
> future.
> 
> Both changes currently breaks the filter ABI, but this should be a
> reasonable compromise since the filtering feature has not been
> released yet.
> 
> Signed-off-by: Christian Babeux <christian.babeux@efficios.com>

Acked-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>

> ---
>  src/bin/lttng-sessiond/lttng-ust-abi.h                      | 6 +++---
>  src/bin/lttng-sessiond/main.c                               | 2 +-
>  src/common/sessiond-comm/sessiond-comm.h                    | 6 +++---
>  src/lib/lttng-ctl/filter/filter-visitor-generate-bytecode.c | 2 +-
>  4 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/src/bin/lttng-sessiond/lttng-ust-abi.h b/src/bin/lttng-sessiond/lttng-ust-abi.h
> index d8b10c2..504c060 100644
> --- a/src/bin/lttng-sessiond/lttng-ust-abi.h
> +++ b/src/bin/lttng-sessiond/lttng-ust-abi.h
> @@ -168,10 +168,10 @@ struct lttng_ust_calibrate {
>  	} u;
>  };
>  
> -#define FILTER_BYTECODE_MAX_LEN		65535
> +#define FILTER_BYTECODE_MAX_LEN		65536
>  struct lttng_ust_filter_bytecode {
> -	uint16_t len;
> -	uint16_t reloc_offset;
> +	uint32_t len;
> +	uint32_t reloc_offset;
>  	char data[0];
>  };
>  
> diff --git a/src/bin/lttng-sessiond/main.c b/src/bin/lttng-sessiond/main.c
> index 318da74..730ac65 100644
> --- a/src/bin/lttng-sessiond/main.c
> +++ b/src/bin/lttng-sessiond/main.c
> @@ -2659,7 +2659,7 @@ skip_domain:
>  	{
>  		struct lttng_filter_bytecode *bytecode;
>  
> -		if (cmd_ctx->lsm->u.filter.bytecode_len > 65336) {
> +		if (cmd_ctx->lsm->u.filter.bytecode_len > LTTNG_FILTER_MAX_LEN) {
>  			ret = LTTNG_ERR_FILTER_INVAL;
>  			goto error;
>  		}
> diff --git a/src/common/sessiond-comm/sessiond-comm.h b/src/common/sessiond-comm/sessiond-comm.h
> index 32ce384..62205f4 100644
> --- a/src/common/sessiond-comm/sessiond-comm.h
> +++ b/src/common/sessiond-comm/sessiond-comm.h
> @@ -208,7 +208,7 @@ struct lttcomm_session_msg {
>  	} u;
>  };
>  
> -#define LTTNG_FILTER_MAX_LEN	65336
> +#define LTTNG_FILTER_MAX_LEN	65536
>  
>  /*
>   * Filter bytecode data. The reloc table is located at the end of the
> @@ -216,8 +216,8 @@ struct lttcomm_session_msg {
>   * starts at reloc_table_offset.
>   */
>  struct lttng_filter_bytecode {
> -	uint16_t len;	/* len of data */
> -	uint16_t reloc_table_offset;
> +	uint32_t len;	/* len of data */
> +	uint32_t reloc_table_offset;
>  	char data[0];
>  };
>  
> diff --git a/src/lib/lttng-ctl/filter/filter-visitor-generate-bytecode.c b/src/lib/lttng-ctl/filter/filter-visitor-generate-bytecode.c
> index 98f8375..332a387 100644
> --- a/src/lib/lttng-ctl/filter/filter-visitor-generate-bytecode.c
> +++ b/src/lib/lttng-ctl/filter/filter-visitor-generate-bytecode.c
> @@ -239,7 +239,7 @@ int visit_node_load(struct filter_parser_ctx *ctx, struct ir_op *node)
>  		uint32_t insn_len = sizeof(struct load_op)
>  			+ sizeof(struct field_ref);
>  		struct field_ref ref_offset;
> -		uint16_t reloc_offset;
> +		uint32_t reloc_offset;
>  
>  		insn = calloc(insn_len, 1);
>  		if (!insn)
> -- 
> 1.7.11.4
> 

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

Re: [PATCH v3 lttng-tools] Filter allocation and large bytecode fixes

[David Goulet]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

Merged!

Christian Babeux:
> Previous iteration of patch 2/3 was missing a typo fix in the 
> LTTNG_FILTER_MAX_LEN define.
> 
> Also, Mathieu suggested that we put a check on the
> reloc_table_offset in the sessiond to check its bounds when
> receiving the LTTNG_SET_FILTER command. This is not necessary
> because the reloc_table_offset does not exist in the filter struct
> of the lttcomm_session_msg union.
> 
> Changelog v3:
> 
> 1/3 - Already merged. 2/3 - Add missing typo fix in
> LTTNG_FILTER_MAX_LEN define. 3/3 - Already merged.
> 
> Changelog v2:
> 
> 1/3 - Already merged. 2/3 - Respin. Support 65536 bytes bytecode
> len + filter ABI changes. 3/3 - Already merged.
> 
> Thanks,
> 
> Christian Babeux (1): Fix: Accept bytecode of length 65536 bytes
> 
> src/bin/lttng-sessiond/lttng-ust-abi.h                      | 6
> +++--- src/bin/lttng-sessiond/main.c
> | 2 +- src/common/sessiond-comm/sessiond-comm.h
> | 6 +++--- 
> src/lib/lttng-ctl/filter/filter-visitor-generate-bytecode.c | 2 +- 
> 4 files changed, 8 insertions(+), 8 deletions(-)
> 
-----BEGIN PGP SIGNATURE-----

iQEcBAEBCgAGBQJQSjDAAAoJEELoaioR9I02atsIAIQtPFOgFQrc4akLIbZTqIOi
AjdzJXkD8HPd4tpYdmLfSLZWi/Ia4rwl2quhWv/+PHOrt5ykBzBApTpjYGzc86oL
2m5ucrK4BFT9sMKU/hLm5eJ2+KVMjTL31Ek9LRt96YmmF2a2QMk/vfAEAJ6VA8XT
3IVfnT9bDNMn/WdwZRP5Dseob38xlumTMN4x/giz4BxBZDM8T4LV+fziFNSdwDvv
ZWros1eSeznUU3uCZ1/85YqYb/CK09LbDv0bD5e+Hv9t4j/WadwyVV/BAuBOh+9m
zBdoZtoxx+Fx6jrGJCNtyeV4/cEDNHfyaVW51ZAyaW4yS7yygQmlvIbs0OJ/SMc=
=F7Wm
-----END PGP SIGNATURE-----