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

[Jérémie Galarneau]

Merged, thanks!

Jérémie

On 15 March 2017 at 10:59, Jonathan Rajotte
<jonathan.rajotte-julien at efficios.com> wrote:
> v3: change commit message to include information regarding off-by-one
> problems induced by 'index' and the use of 'index' as value of
> syscall_table_nb_entry.
>
> --
>
> 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
> syscall_table_nb_entry value and an off-by-one error on table memory
> reallocation.
>
> The index value returned by fscanf is an index starting at 0. It is
> later assigned to syscall_table_nb_entry which is used for memory
> allocation and iteration during syscall_table_list. Forgetting to add 1
> results in losing the last syscall during listing.
>
> The parsed index value is also used to count how many elements should be
> allocated during table reallocation, without any extra increment which
> result in an off-by-one error. Hence, make sure to increment its value by
> one when assigning the value of syscall_table_nb_entry. 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).
>
> 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 at 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
>



-- 
Jérémie Galarneau
EfficiOS Inc.
http://www.efficios.com