From 6123e5aea63e669b9df73f7fa287e27ad28db426 Mon Sep 17 00:00:00 2001 From: Mattias Nissler Date: Tue, 11 Feb 2020 13:38:03 +0100 Subject: Improve resource management for minijail_run_internal Previously, the code was tracking resources like file descriptors in local variables, which could leak when exiting via error paths. Improve this by introducing a struct to hold state. With this in place, we can also break out the code to grab file descriptors to pass back to the caller into a wrapper function, thus simplifying minijail_run_internal. Furthermore, additional resources (such as allocated child environments, which are subject of a subsequent code change) can now be added in a straightforward way. No (intended) functional changes. BUG=chromium:1050997 TEST=Builds and passes unit tests and security.Minijail* tast tests. Change-Id: Ic80cbc92c428b3d0346768cd594e98faf7cc60a2 --- system_unittest.cc | 36 ------------------------------------ 1 file changed, 36 deletions(-) (limited to 'system_unittest.cc') diff --git a/system_unittest.cc b/system_unittest.cc index e13630a..97c1d4e 100644 --- a/system_unittest.cc +++ b/system_unittest.cc @@ -157,42 +157,6 @@ TEST(cap_ambient_supported, smoke) { cap_ambient_supported(); } -// Invalid indexes should return errors, not crash. -TEST(setup_pipe_end, bad_index) { - EXPECT_LT(setup_pipe_end(nullptr, 2), 0); - EXPECT_LT(setup_pipe_end(nullptr, 3), 0); - EXPECT_LT(setup_pipe_end(nullptr, 4), 0); -} - -// Verify getting the first fd works. -TEST(setup_pipe_end, index0) { - int fds[2]; - EXPECT_EQ(0, pipe(fds)); - // This should close fds[1] and return fds[0]. - EXPECT_EQ(fds[0], setup_pipe_end(fds, 0)); - // Use close() to verify open/close state. - EXPECT_EQ(-1, close(fds[1])); - EXPECT_EQ(0, close(fds[0])); -} - -// Verify getting the second fd works. -TEST(setup_pipe_end, index1) { - int fds[2]; - EXPECT_EQ(0, pipe(fds)); - // This should close fds[0] and return fds[1]. - EXPECT_EQ(fds[1], setup_pipe_end(fds, 1)); - // Use close() to verify open/close state. - EXPECT_EQ(-1, close(fds[0])); - EXPECT_EQ(0, close(fds[1])); -} - -// Invalid indexes should return errors, not crash. -TEST(dupe_and_close_fd, bad_index) { - EXPECT_LT(dupe_and_close_fd(nullptr, 2, -1), 0); - EXPECT_LT(dupe_and_close_fd(nullptr, 3, -1), 0); - EXPECT_LT(dupe_and_close_fd(nullptr, 4, -1), 0); -} - // An invalid path should return an error. TEST(write_pid_to_path, bad_path) { EXPECT_NE(0, write_pid_to_path(0, kNoSuchDir)); -- cgit v1.2.3