[lxc-devel] [PATCH] lxc-ls: try to protect stack in recursive function
Serge Hallyn
serge.hallyn at ubuntu.com
Wed Jan 13 22:28:30 UTC 2016
Quoting Christian Brauner (christian.brauner at mailbox.org):
> As ls_get() is non-tail recursive we face the inherent danger of blowing up the
> stack at some level of nesting. To have at least some security we define
> MAX_NESTLVL to be 5. That should be sufficient for most users. The argument lvl
> to ls_get() can be used to keep track of the level of nesting we are at. If lvl
> is greater than the allowed default level return (without error) and unwind the
> stack.
>
> --nesting gains an optional numeric argument. This allows the user to specify
> the maximum level of nesting she/he wants to see. Fair warning: If your nesting
> level is really deep and/or you have a lot of containers your might run into
> trouble.
>
> Signed-off-by: Christian Brauner <christian.brauner at mailbox.org>
Acked-by: Serge E. Hallyn <serge.hallyn at ubuntu.com>
> ---
> src/lxc/arguments.h | 2 +-
> src/lxc/lxc_ls.c | 37 ++++++++++++++++++++++++++++++++++---
> 2 files changed, 35 insertions(+), 4 deletions(-)
>
> diff --git a/src/lxc/arguments.h b/src/lxc/arguments.h
> index 898a9ce..a3d2932 100644
> --- a/src/lxc/arguments.h
> +++ b/src/lxc/arguments.h
> @@ -124,11 +124,11 @@ struct lxc_arguments {
> char *ls_fancy_format;
> char *ls_groups;
> char *ls_regex;
> + unsigned int ls_nesting; /* maximum allowed nesting level */
> bool ls_active;
> bool ls_fancy;
> bool ls_frozen;
> bool ls_line;
> - bool ls_nesting;
> bool ls_running;
> bool ls_stopped;
>
> diff --git a/src/lxc/lxc_ls.c b/src/lxc/lxc_ls.c
> index 3a7fa15..a4ab9b6 100644
> --- a/src/lxc/lxc_ls.c
> +++ b/src/lxc/lxc_ls.c
> @@ -40,6 +40,10 @@
> lxc_log_define(lxc_ls, lxc);
>
> #define LINELEN 1024
> +/* Per default we only allow five levels of recursion to protect the stack at
> + * least a little bit. */
> +#define MAX_NESTLVL 5
> +
> #define LS_FROZEN 1
> #define LS_STOPPED 2
> #define LS_ACTIVE 3
> @@ -154,7 +158,7 @@ static const struct option my_longopts[] = {
> {"running", no_argument, 0, LS_RUNNING},
> {"frozen", no_argument, 0, LS_FROZEN},
> {"stopped", no_argument, 0, LS_STOPPED},
> - {"nesting", no_argument, 0, LS_NESTING},
> + {"nesting", optional_argument, 0, LS_NESTING},
> {"groups", required_argument, 0, 'g'},
> {"regex", required_argument, 0, 'r'},
> LXC_COMMON_OPTIONS
> @@ -177,11 +181,12 @@ Options :\n\
> --running list only running containers\n\
> --frozen list only frozen containers\n\
> --stopped list only stopped containers\n\
> - --nesting list nested containers\n\
> + --nesting=NUM list nested containers up to NUM (default is 5) levels of nesting\n\
> -r --regex filter container names by regular expression\n\
> -g --groups comma separated list of groups a container must have to be displayed\n",
> .options = my_longopts,
> .parser = my_parser,
> + .ls_nesting = MAX_NESTLVL,
> };
>
> int main(int argc, char *argv[])
> @@ -300,6 +305,16 @@ static int ls_get(struct ls **m, size_t *size, const struct lxc_arguments *args,
> struct lengths *lht, const char *basepath, const char *parent,
> unsigned int lvl)
> {
> + /* As ls_get() is non-tail recursive we face the inherent danger of
> + * blowing up the stack at some level of nesting. To have at least some
> + * security we define MAX_NESTLVL to be 5. That should be sufficient for
> + * most users. The argument lvl can be used to keep track of the level
> + * of nesting we are at. If lvl is greater than the allowed default
> + * level or the level the user specified on the command line we return
> + * and unwind the stack. */
> + if (lvl > args->ls_nesting)
> + return 0;
> +
> int num = 0, ret = -1;
> char **containers = NULL;
> /* If we, at some level of nesting, encounter a stopped container but
> @@ -855,6 +870,8 @@ static void ls_print_table(struct ls *l, struct lengths *lht,
>
> static int my_parser(struct lxc_arguments *args, int c, char *arg)
> {
> + char *invalid;
> + unsigned long int m, n = MAX_NESTLVL;
> switch (c) {
> case '1':
> args->ls_line = true;
> @@ -875,7 +892,21 @@ static int my_parser(struct lxc_arguments *args, int c, char *arg)
> args->ls_stopped = true;
> break;
> case LS_NESTING:
> - args->ls_nesting = true;
> + /* In case strtoul() receives a string that represents a
> + * negative number it will return ULONG_MAX - the number that
> + * string represents if the number the string represents is <
> + * ULONG_MAX and ULONG_MAX otherwise. But it will consider this
> + * valid input and not set errno. So we check manually if the
> + * first character of num_string == '-'. Otherwise the default
> + * level remains set. */
> + if (arg && !(*arg == '-')) {
> + errno = 0;
> + m = strtoul(arg, &invalid, 0);
> + /* ls_nesting has type unsigned int. */
> + if (!errno && (*invalid == '\0') && (m <= UINT_MAX))
> + n = m;
> + }
> + args->ls_nesting = n;
> break;
> case 'g':
> args->groups = arg;
> --
> 2.7.0
>
More information about the lxc-devel
mailing list