Skip to content

Add an Option to Limit the Sandbox Child Process Memory #192

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
turbo wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add an Option to Limit the Sandbox Child Process Memory #192

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions bubblewrap.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
#include <linux/sched.h>
#include <linux/seccomp.h>
#include <linux/filter.h>
#include <sys/resource.h>

#include "utils.h"
#include "network.h"
Expand Down Expand Up @@ -72,6 +73,7 @@ int opt_sync_fd = -1;
int opt_block_fd = -1;
int opt_info_fd = -1;
int opt_seccomp_fd = -1;
int opt_limit_mem = 0;
char *opt_sandbox_hostname = NULL;

typedef enum {
Expand Down Expand Up @@ -219,6 +221,7 @@ usage (int ecode, FILE *out)
" --info-fd FD Write information about the running container to FD\n"
" --new-session Create a new terminal session\n"
" --die-with-parent Kills with SIGKILL child process (COMMAND) when bwrap or bwrap's parent dies.\n"
" --limit-mem VALUE Limit the sandbox' virtual memory allocation to VALUE MB.\n"
);
exit (ecode);
}
Expand Down Expand Up @@ -1652,6 +1655,23 @@ parse_args_recurse (int *argcp,
{
opt_die_with_parent = TRUE;
}
else if (strcmp (arg, "--limit-mem") == 0)
{
int target_limit;
char *endptr;

if (argc < 2)
die ("--limit-mem takes an argument");

target_limit = strtol (argv[1], &endptr, 10);
if (argv[1][0] == 0 || endptr[0] != 0 || target_limit < 1)
die ("Invalid memory size: %s", argv[1]);

opt_limit_mem = target_limit;

argv += 1;
argc -= 1;
}
else if (*arg == '-')
{
die ("Unknown option %s", arg);
Expand Down Expand Up @@ -1721,6 +1741,7 @@ main (int argc,
cleanup_free char *seccomp_data = NULL;
size_t seccomp_len;
struct sock_fprog seccomp_prog;
struct rlimit limit_mem;

/* Handle --version early on before we try to acquire/drop
* any capabilities so it works in a build environment;
Expand Down Expand Up @@ -2216,6 +2237,13 @@ main (int argc,
prctl (PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &seccomp_prog) != 0)
die_with_error ("prctl(PR_SET_SECCOMP)");

/* Optionally limit memory. This must occur after the fork and
* before the exec* call. */
if (opt_limit_mem) {
Copy link
Copy Markdown
Collaborator

@cgwalters cgwalters Apr 25, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to come before the seccomp filter - see the rationale in #155

Copy link
Copy Markdown
Collaborator

@cgwalters cgwalters Apr 25, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, code style is GNU - braces on a new line.

limit_mem.rlim_cur = limit_mem.rlim_max = opt_limit_mem * 1048576;
setrlimit(RLIMIT_DATA, &limit_mem);
Copy link
Copy Markdown
Collaborator

@cgwalters cgwalters Apr 25, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And yeah, we should probably just disable this in the setuid case...unless it seems easy to get the current limit and check it ourself. (Alternatively, maybe we could temporarily drop CAP_SYS_RESOURCE before doing this call? Actually, maybe we're dropping it already?)

Copy link
Copy Markdown
Author

@turbo turbo Apr 29, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to go with the safer method first, denying this option in setuid mode. I don't know if there's a way to reliably check the limit for all supported platforms. That seems like something that's less stable and could be done in the future.

Checking the cap is a better way. I'll investigate.

}

if (execvp (argv[0], argv) == -1)
die_with_error ("execvp %s", argv[0]);

Expand Down