Re: [PATCH lttng-tools v2] Fix: syscall_table_nb_entry invalid value when no syscalls TPs are defined

[Mathieu Desnoyers <mathieu.desnoyers@efficios.com>]

----- On Mar 14, 2017, at 10:37 AM, Jonathan Rajotte jonathan.rajotte-julien@efficios.com wrote:

> v2: merged previous series in one patch since the second patch is a minor fix
> and partly addressed in the first patch.
> 
> "start" -> starts
> 
> --
> 
> fscanf on an empty file returns directly without assigning value to
> 'index' leading to assigning the value of an uninitialized variable to
> syscall_table_nb_entry. This can result in memory allocation problems
> when listing syscalls on 'lttng list --kernel --syscall'[1][2].
> 
> Fixes at the same time an off-by-one problem for the index
> value.

Fixing this off-by-one also fixes the missing last element issue. It should
be described in this changelog, but it's not. Only the effect on memory
allocation is discussed.

Thanks,

Mathieu

> 
> The index value returned by fscanf is an index starting at 0.
> However, it is later used to count how many elements need to be
> allocated, without any extra increment.
> 
> It does not cause issues in practice because SYSCALL_TABLE_INIT_SIZE
> is nonzero, and because we don't require the table to expand by more
> than the double of its size at once (which could happen if we could
> have a hole in the syscall table for instance).
> 
> It's better to fix this off-by-one for clarity, and in case this code
> gets re-used in other contexts that don't have the same memory growth
> pattern.
> 
> Fixes #1091
> 
> [1] https://bugs.lttng.org/issues/1091
> [2] https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1671063/
> 
> Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
> ---
> src/bin/lttng-sessiond/syscall.c | 15 +++++++++++----
> 1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/src/bin/lttng-sessiond/syscall.c b/src/bin/lttng-sessiond/syscall.c
> index 6ee38bd..7d0a92b 100644
> --- a/src/bin/lttng-sessiond/syscall.c
> +++ b/src/bin/lttng-sessiond/syscall.c
> @@ -16,6 +16,8 @@
>  */
> 
> #define _LGPL_SOURCE
> +#include <stdbool.h>
> +
> #include <common/bitfield.h>
> #include <common/common.h>
> #include <common/kernel-ctl/kernel-ctl.h>
> @@ -43,7 +45,8 @@ int syscall_init_table(void)
> 	size_t nbmem;
> 	FILE *fp;
> 	/* Syscall data from the kernel. */
> -	size_t index;
> +	size_t index = 0;
> +	bool at_least_one_syscall = false;
> 	uint32_t bitness;
> 	char name[SYSCALL_NAME_LEN];
> 
> @@ -76,12 +79,13 @@ int syscall_init_table(void)
> 				name = %" XSTR(SYSCALL_NAME_LEN) "[^;]; \
> 				bitness = %u; };\n",
> 				&index, name, &bitness) == 3) {
> -		if (index >= nbmem ) {
> +		at_least_one_syscall = true;
> +		if (index >= nbmem) {
> 			struct syscall *new_list;
> 			size_t new_nbmem;
> 
> 			/* Double memory size. */
> -			new_nbmem = max(index, nbmem << 1);
> +			new_nbmem = max(index + 1, nbmem << 1);
> 			if (new_nbmem > (SIZE_MAX / sizeof(*new_list))) {
> 				/* Overflow, stop everything, something went really wrong. */
> 				ERR("Syscall listing memory size overflow. Stopping");
> @@ -123,7 +127,10 @@ int syscall_init_table(void)
> 		*/
> 	}
> 
> -	syscall_table_nb_entry = index;
> +	/* Index starts at 0. */
> +	if (at_least_one_syscall) {
> +		syscall_table_nb_entry = index + 1;
> +	}
> 
> 	ret = 0;
> 
> --
> 2.7.4
> 
> _______________________________________________
> lttng-dev mailing list
> lttng-dev@lists.lttng.org
> https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev