Richard W.M. Jones
2015-May-26 12:34 UTC
[Libguestfs] [PATCH] lib: Limit space and time used by 'qemu-img info' subprocess.
After fuzzing 'qemu-img info' I found that certain files can cause the command to use lots of memory and time. Modify the command mini-library to allow us to place resource limits on subprocesses, and use these to limit the amount of space and time used by 'qemu-img info'. --- configure.ac | 3 +++ src/command.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++ src/guestfs-internal.h | 1 + src/info.c | 22 ++++++++++++++++++++- 4 files changed, 78 insertions(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index acd807b..6fea1e4 100644 --- a/configure.ac +++ b/configure.ac @@ -305,8 +305,10 @@ AC_CHECK_HEADERS([\ linux/raid/md_u.h \ printf.h \ sys/inotify.h \ + sys/resource.h \ sys/socket.h \ sys/statvfs.h \ + sys/time.h \ sys/types.h \ sys/un.h \ sys/wait.h \ @@ -335,6 +337,7 @@ AC_CHECK_FUNCS([\ posix_fadvise \ removexattr \ setitimer \ + setrlimit \ setxattr \ sigaction \ statvfs \ diff --git a/src/command.c b/src/command.c index 45ae5d6..993198a 100644 --- a/src/command.c +++ b/src/command.c @@ -77,6 +77,13 @@ #include <sys/wait.h> #include <sys/select.h> +#ifdef HAVE_SYS_TIME_H +#include <sys/time.h> +#endif +#ifdef HAVE_SYS_RESOURCE_H +#include <sys/resource.h> +#endif + #include "guestfs.h" #include "guestfs-internal.h" @@ -101,6 +108,12 @@ struct buffering { void (*close_data) (struct command *cmd); }; +struct child_rlimits { + struct child_rlimits *next; + int resource; + long limit; +}; + struct command { guestfs_h *g; @@ -139,6 +152,9 @@ struct command cmd_child_callback child_callback; void *child_callback_data; + /* Optional child limits. */ + struct child_rlimits *child_rlimits; + /* Optional stdin forwarding to the child. */ int infd; }; @@ -329,6 +345,22 @@ guestfs_int_cmd_set_child_callback (struct command *cmd, cmd->child_callback_data = data; } +/* Set up child rlimits, in case the process we are running could + * consume lots of space or time. + */ +void +guestfs_int_cmd_set_child_rlimit (struct command *cmd, int resource, long limit) +{ + struct child_rlimits *p; + + p = safe_malloc (cmd->g, sizeof *p); + p->resource = resource; + p->limit = limit; + p->next = cmd->child_rlimits; + cmd->child_rlimits = p; +} + + /* Finish off the command by either NULL-terminating the argv array or * adding a terminating \0 to the string, or die with an internal * error if no command has been added. @@ -390,6 +422,10 @@ run_command (struct command *cmd, bool get_stdin_fd, bool get_stdout_fd, int outfd[2] = { -1, -1 }; int infd[2] = { -1, -1 }; char status_string[80]; +#ifdef HAVE_SETRLIMIT + struct child_rlimits *child_rlimit; + struct rlimit rlimit; +#endif get_stdout_fd = get_stdout_fd || cmd->stdout_callback != NULL; get_stderr_fd = get_stderr_fd || cmd->capture_errors; @@ -510,6 +546,23 @@ run_command (struct command *cmd, bool get_stdin_fd, bool get_stdout_fd, _exit (EXIT_FAILURE); } +#ifdef HAVE_SETRLIMIT + for (child_rlimit = cmd->child_rlimits; + child_rlimit != NULL; + child_rlimit = child_rlimit->next) { + rlimit.rlim_cur = rlimit.rlim_max = child_rlimit->limit; + if (setrlimit (child_rlimit->resource, &rlimit) == -1) { + /* EPERM means we're trying to raise the limit (ie. the limit is + * already more restrictive than what we want), so ignore it. + */ + if (errno != EPERM) { + perror ("setrlimit"); + _exit (EXIT_FAILURE); + } + } + } +#endif /* HAVE_SETRLIMIT */ + /* Run the command. */ switch (cmd->style) { case COMMAND_STYLE_EXECV: diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h index 414a634..4f06c37 100644 --- a/src/guestfs-internal.h +++ b/src/guestfs-internal.h @@ -864,6 +864,7 @@ extern void guestfs_int_cmd_set_stdout_callback (struct command *, cmd_stdout_ca #define CMD_STDOUT_FLAG_UNBUFFERED 1 #define CMD_STDOUT_FLAG_WHOLE_BUFFER 2 extern void guestfs_int_cmd_set_stderr_to_stdout (struct command *); +extern void guestfs_int_cmd_set_child_rlimit (struct command *, int resource, long limit); extern void guestfs_int_cmd_clear_capture_errors (struct command *); extern void guestfs_int_cmd_clear_close_files (struct command *); extern void guestfs_int_cmd_set_child_callback (struct command *, cmd_child_callback child_callback, void *data); diff --git a/src/info.c b/src/info.c index bd4221c..bfd7860 100644 --- a/src/info.c +++ b/src/info.c @@ -31,6 +31,14 @@ #include <assert.h> #include <string.h> +#ifdef HAVE_SYS_TIME_H +#include <sys/time.h> +#endif + +#ifdef HAVE_SYS_RESOURCE_H +#include <sys/resource.h> +#endif + #if HAVE_YAJL #include <yajl/yajl_tree.h> #endif @@ -269,7 +277,13 @@ get_json_output (guestfs_h *g, const char *filename) guestfs_int_cmd_add_arg (cmd, "json"); guestfs_int_cmd_add_arg (cmd, fdpath); guestfs_int_cmd_set_stdout_callback (cmd, parse_json, &tree, - CMD_STDOUT_FLAG_WHOLE_BUFFER); + CMD_STDOUT_FLAG_WHOLE_BUFFER); +#ifdef RLIMIT_AS + guestfs_int_cmd_set_child_rlimit (cmd, RLIMIT_AS, 1000000000 /* 1GB */); +#endif +#ifdef RLIMIT_CPU + guestfs_int_cmd_set_child_rlimit (cmd, RLIMIT_CPU, 10 /* seconds */); +#endif r = guestfs_int_cmd_run (cmd); close (fd); if (r == -1) @@ -548,6 +562,12 @@ old_parser_run_qemu_img_info (guestfs_h *g, const char *filename, guestfs_int_cmd_add_arg (cmd, "info"); guestfs_int_cmd_add_arg (cmd, safe_filename); guestfs_int_cmd_set_stdout_callback (cmd, fn, data, 0); +#ifdef RLIMIT_AS + guestfs_int_cmd_set_child_rlimit (cmd, RLIMIT_AS, 1000000000 /* 1GB */); +#endif +#ifdef RLIMIT_CPU + guestfs_int_cmd_set_child_rlimit (cmd, RLIMIT_CPU, 10 /* seconds */); +#endif r = guestfs_int_cmd_run (cmd); if (r == -1) return -1; -- 2.3.1
Apparently Analagous Threads
- [PATCH 1/2] lib: info: Move common code for setting child rlimits.
- [libguestfs PATCH] lib: remove guestfs_int_cmd_clear_close_files()
- [PATCH v2 1/2] lib: command: If command fails, exit with 126 or 127 as defined by POSIX.
- [PATCH 0/7] copy-in/copy-out: Capture errors from tar subprocess (RHBZ#1267032).
- [PATCH 1/6] cmd: add a way to run (and wait) asynchronously commands