Ensure rss_limit_mb is set to 0 for all Chromium/Google tasks

src/clusterfuzz/_internal/bot/fuzzers/centipede/engine.py

@@ -630,6 +630,8 @@ def _get_smallest_crasher(self, workdir_path):
     minimum_testcase = min(testcases, key=os.path.getsize)
     return minimum_testcase
 
+  # FIXME(crbug.com/564526622): Investigate whether `minimize_testcase` needs
+  # to be used, or if it can be safely deleted
   def minimize_testcase(self, target_path, arguments, input_path, output_path,
                         max_time):
     """Minimizes a testcase.
@@ -647,7 +649,14 @@ def minimize_testcase(self, target_path, arguments, input_path, output_path,
     runner = _get_runner(target_path)
     workdir = engine_common.create_temp_fuzzing_dir('workdir')
     timeout = max_time + _CLEAN_EXIT_SECS
-    args = [
+
+    # Centipede does not store arguments in the database, so the `arguments`
+    # parameter is safe to ignore
+    minimize_arguments = self._get_arguments(target_path)
+    self._strip_fuzzing_arguments(minimize_arguments)
+    # Remove `TIMEOUT_PER_INPUT` flag set by `_get_arguments`
+    del minimize_arguments[constants.TIMEOUT_PER_INPUT_FLAGNAME]
+    args = minimize_arguments.list() + [
         f'--binary={target_path}',
         f'--workdir={workdir}',
         f'--minimize_crash={input_path}',

src/clusterfuzz/_internal/bot/fuzzers/libFuzzer/engine.py

@@ -98,6 +98,24 @@ def fuzz_additional_processing_timeout(self, options):
     # get_fuzz_timeout returns a negative value.
     return -fuzz_timeout
 
+  def _add_rss_limit_if_missing(self, target_path, arguments):
+    """Add rss_limit_mb from target arguments if missing. Returns arguments
+    unchanged if it cannot be parsed or if rss_limit_mb is already present.
+    """
+    db_arguments = fuzzer_options.FuzzerArguments.from_list(arguments)
+    if db_arguments is None:
+      logs.warning(
+          'Failed to parse arguments. Returning without setting rss_limit_mb.',
+          arguments=arguments)
+      return arguments
+
+    if constants.RSS_LIMIT_FLAGNAME in db_arguments:
+      return arguments
+
+    db_arguments[constants.RSS_LIMIT_FLAGNAME] = fuzzer.get_rss_limit_mb(
+        fuzzer_options.get_fuzz_target_options(target_path))
+    return db_arguments.list()
+
   def prepare(self, corpus_dir, target_path, build_dir):
     """Prepare for a fuzzing session, by generating options. Returns a
     FuzzOptions object.
@@ -397,6 +415,7 @@ def reproduce(self, target_path, input_path, arguments, max_time):
     # Remove fuzzing specific arguments. This is only really needed for legacy
     # testcases, and can be removed in the distant future.
     arguments = libfuzzer.strip_fuzzing_arguments(arguments)
+    arguments = self._add_rss_limit_if_missing(target_path, arguments)
     arguments = fuzzer_options.FuzzerArguments.from_list(arguments)
 
     arguments[constants.RUNS_FLAGNAME] = int(constants.RUNS_TO_REPRODUCE)
@@ -514,6 +533,7 @@ def minimize_corpus(self, target_path, arguments, input_dirs, output_dir,
     """
     runner = libfuzzer.get_runner(target_path)
     libfuzzer.set_sanitizer_options(target_path)
+    arguments = self._add_rss_limit_if_missing(target_path, arguments)
     merge_tmp_dir = self._create_temp_dir('merge-wd')
     logs.info(f'Starting merge with timeout {max_time}.')
 
@@ -567,6 +587,8 @@ def minimize_testcase(self, target_path, arguments, input_path, output_path,
     runner = libfuzzer.get_runner(target_path)
     libfuzzer.set_sanitizer_options(target_path)
 
+    arguments = self._add_rss_limit_if_missing(target_path, arguments)
+
     minimize_tmp_dir = engine_common.create_temp_fuzzing_dir('minimize-workdir')
     result = runner.minimize_crash(
         input_path,
@@ -601,6 +623,8 @@ def cleanse(self, target_path, arguments, input_path, output_path, max_time):
     runner = libfuzzer.get_runner(target_path)
     libfuzzer.set_sanitizer_options(target_path)
 
+    arguments = self._add_rss_limit_if_missing(target_path, arguments)
+
     cleanse_tmp_dir = engine_common.create_temp_fuzzing_dir('cleanse-workdir')
     result = runner.cleanse_crash(
         input_path,

src/clusterfuzz/_internal/bot/fuzzers/libFuzzer/fuzzer.py

@@ -29,38 +29,43 @@ def get_extra_env(fuzzer_path):
   return None
 
 
+def get_rss_limit_mb(fuzzer_options=None) -> int:
+  """Returns the rss_limit_mb to use for a target with the given options."""
+  rss_limit_mb = None
+  if fuzzer_options:
+    rss_limit_mb = fuzzer_options.get_engine_arguments('libfuzzer').get(
+        'rss_limit_mb', constructor=int)
+
+  if rss_limit_mb is None:
+    if utils.is_chromium() or utils.default_project_name() == 'google':
+      return 0
+    return constants.DEFAULT_RSS_LIMIT_MB
+
+  # psutil gives the total amount of memory in bytes, but we're only dealing
+  # with options that are counting memory space in MB, so we need to do the
+  # conversion first.
+  max_memory_limit_mb = (psutil.virtual_memory().total //
+                         (1 << 20)) - constants.MEMORY_OVERHEAD
+  # Custom rss_limit_mb value shouldn't be greater than the actual memory
+  # allocated on the machine.
+  return min(rss_limit_mb, max_memory_limit_mb)
+
+
 def get_arguments(fuzzer_path) -> options.FuzzerArguments:
   """Get arguments for a given fuzz target."""
   arguments = options.FuzzerArguments()
-  rss_limit_mb = None
   timeout = None
 
   fuzzer_options = options.get_fuzz_target_options(fuzzer_path)
 
   if fuzzer_options:
     arguments = fuzzer_options.get_engine_arguments('libfuzzer')
-    rss_limit_mb = arguments.get('rss_limit_mb', constructor=int)
     timeout = arguments.get('timeout', constructor=int)
 
   if timeout is None:
     arguments[constants.TIMEOUT_FLAGNAME] = constants.DEFAULT_TIMEOUT_LIMIT
 
-  if not rss_limit_mb and (utils.is_chromium() or
-                           utils.default_project_name() == 'google'):
-    # TODO(metzman/alhijazi): Monitor if we are crashing the bots.
-    arguments[constants.RSS_LIMIT_FLAGNAME] = 0
-  elif not rss_limit_mb:
-    arguments[constants.RSS_LIMIT_FLAGNAME] = constants.DEFAULT_RSS_LIMIT_MB
-  else:
-    # psutil gives the total amount of memory in bytes, but we're only dealing
-    # with options that are counting memory space in MB, so we need to do the
-    # conversion first.
-    max_memory_limit_mb = (psutil.virtual_memory().total //
-                           (1 << 20)) - constants.MEMORY_OVERHEAD
-    # Custom rss_limit_mb value shouldn't be greater than the actual memory
-    # allocated on the machine.
-    if rss_limit_mb > max_memory_limit_mb:
-      arguments[constants.RSS_LIMIT_FLAGNAME] = max_memory_limit_mb
+  arguments[constants.RSS_LIMIT_FLAGNAME] = get_rss_limit_mb(fuzzer_options)
 
   return arguments
 

src/clusterfuzz/_internal/bot/tasks/utasks/corpus_pruning_task.py

@@ -33,6 +33,7 @@
 from clusterfuzz._internal.bot.fuzzers import engine_common
 from clusterfuzz._internal.bot.fuzzers import options
 from clusterfuzz._internal.bot.fuzzers.libFuzzer import constants
+from clusterfuzz._internal.bot.fuzzers.libFuzzer import fuzzer
 from clusterfuzz._internal.bot.tasks import setup
 from clusterfuzz._internal.bot.tasks import task_creation
 from clusterfuzz._internal.bot.tasks.utasks import uworker_handle_errors
@@ -87,10 +88,6 @@
 # Maximum number of units to restore from quarantine in one run.
 MAX_QUARANTINE_UNITS_TO_RESTORE = 128
 
-# Memory limits for testcase.
-RSS_LIMIT = 2560
-RSS_LIMIT_MB_FLAG = '-rss_limit_mb=%d'
-
 # Flag to enforce length limit for a single corpus element.
 MAX_LEN_FLAG = '-max_len=%d'
 
@@ -400,22 +397,15 @@ class LibFuzzerRunner(BaseRunner):
 
   def get_fuzzer_flags(self):
     """Get default libFuzzer options for pruning."""
-    rss_limit = RSS_LIMIT
+    rss_limit = fuzzer.get_rss_limit_mb(self.fuzzer_options)
+
     max_len = engine_common.CORPUS_INPUT_SIZE_LIMIT
     detect_leaks = 1
-    arguments = options.FuzzerArguments()
-    arguments[constants.TIMEOUT_FLAGNAME] = SINGLE_UNIT_TIMEOUT
-
     if self.fuzzer_options:
       # Default values from above can be customized for a given fuzz target.
       libfuzzer_arguments = self.fuzzer_options.get_engine_arguments(
           'libfuzzer')
 
-      custom_rss_limit = libfuzzer_arguments.get(
-          'rss_limit_mb', constructor=int)
-      if custom_rss_limit:
-        rss_limit = custom_rss_limit
-
       custom_max_len = libfuzzer_arguments.get('max_len', constructor=int)
       if custom_max_len and custom_max_len < max_len:
         max_len = custom_max_len
@@ -427,10 +417,13 @@ def get_fuzzer_flags(self):
       if custom_detect_leaks is not None:
         detect_leaks = custom_detect_leaks
 
-    arguments[constants.RSS_LIMIT_FLAGNAME] = rss_limit
-    arguments[constants.MAX_LEN_FLAGNAME] = max_len
-    arguments[constants.DETECT_LEAKS_FLAGNAME] = detect_leaks
-    arguments[constants.VALUE_PROFILE_FLAGNAME] = 1
+    arguments = options.FuzzerArguments({
+        constants.TIMEOUT_FLAGNAME: SINGLE_UNIT_TIMEOUT,
+        constants.RSS_LIMIT_FLAGNAME: rss_limit,
+        constants.MAX_LEN_FLAGNAME: max_len,
+        constants.DETECT_LEAKS_FLAGNAME: detect_leaks,
+        constants.VALUE_PROFILE_FLAGNAME: 1,
+    })
 
     return arguments.list()
 

src/clusterfuzz/_internal/tests/core/bot/fuzzers/centipede/centipede_engine_test.py

@@ -220,6 +220,31 @@ def test_options_arguments(self):
     args = arguments.list()
     self.assertIn('-rss_limit_mb=1234', args)
 
+  @patch('clusterfuzz._internal.base.utils.is_chromium', return_value=True)
+  def test_options_arguments_chromium_default(self, _):
+    """Tests that rss_limit_mb=0 on Chromium when unspecified."""
+    testcase_path = setup_testcase('test_fuzzer', self.test_paths)
+    engine_impl = engine.Engine()
+    # pylint: disable=protected-access
+    arguments = engine_impl._get_arguments(str(testcase_path))
+    args = arguments.list()
+    self.assertIn('-rss_limit_mb=0', args)
+
+  @patch('clusterfuzz._internal.base.utils.is_chromium', return_value=False)
+  @patch(
+      'clusterfuzz._internal.base.utils.default_project_name',
+      return_value='test-project')
+  def test_options_arguments_non_chromium_default(self, unused_default_project,
+                                                  unused_is_chromium):
+    """Tests that rss_limit_mb defaults to RSS_LIMIT_MB_DEFAULT on non-Chromium."""
+    testcase_path = setup_testcase('test_fuzzer', self.test_paths)
+    engine_impl = engine.Engine()
+    # pylint: disable=protected-access
+    arguments = engine_impl._get_arguments(str(testcase_path))
+    args = arguments.list()
+    self.assertIn(f'-rss_limit_mb={centipede_constants.RSS_LIMIT_MB_DEFAULT}',
+                  args)
+
   @patch('clusterfuzz._internal.bot.fuzzers.centipede.engine._CLEAN_EXIT_SECS',
          5)
   def _run_centipede(self,

src/clusterfuzz/_internal/tests/core/bot/fuzzers/libFuzzer/engine_test.py

@@ -142,6 +142,13 @@ def test_prepare_auto_add_dict(self):
         '-dict=/path/target.dict'
     ], options.arguments)
 
+  @mock.patch('clusterfuzz._internal.base.utils.is_chromium', return_value=True)
+  def test_prepare_chromium(self, _):
+    """Test prepare on Chromium defaults rss_limit_mb to 0."""
+    engine_impl = engine.Engine()
+    options = engine_impl.prepare('/corpus_dir', '/path/target', '/path')
+    self.assertIn('-rss_limit_mb=0', options.arguments)
+
 
 class FuzzAdditionalProcessingTimeoutTest(unittest.TestCase):
   """fuzz_additional_processing_timeout tests."""
@@ -442,6 +449,77 @@ def mock_get_directory_file_count(dir_path):
   return _get_directory_file_count_orig(dir_path)
 
 
+class EngineArgumentsTest(unittest.TestCase):
+  """Tests that Engine methods default and pass arguments properly."""
+
+  def setUp(self):
+    test_helpers.patch_environ(self)
+    test_helpers.patch(self, [
+        'clusterfuzz._internal.bot.fuzzers.libfuzzer.get_runner',
+        'clusterfuzz._internal.bot.fuzzers.libfuzzer.set_sanitizer_options',
+        'clusterfuzz._internal.bot.fuzzers.libFuzzer.fuzzer.get_rss_limit_mb',
+        'clusterfuzz._internal.bot.fuzzers.options.get_fuzz_target_options',
+    ])
+    self.runner = mock.MagicMock()
+    self.mock.get_runner.return_value = self.runner
+    self.mock.get_rss_limit_mb.return_value = 0
+    self.engine = engine.Engine()
+
+  def test_reproduce_defaults_rss_limit(self):
+    """Test that reproduce defaults rss_limit_mb when missing."""
+    self.runner.run_single_testcase.return_value = new_process.ProcessResult(
+        command=['/target'], return_code=0, output='', time_executed=1)
+    self.engine.reproduce('/target', '/input', ['-some_arg=1'], 30)
+    self.runner.run_single_testcase.assert_called_once_with(
+        '/input',
+        timeout=30,
+        additional_args=['-some_arg=1', '-rss_limit_mb=0', '-runs=100'])
+
+  def test_reproduce_preserves_existing_rss_limit(self):
+    """Test that reproduce preserves existing rss_limit_mb."""
+    self.runner.run_single_testcase.return_value = new_process.ProcessResult(
+        command=['/target'], return_code=0, output='', time_executed=1)
+    self.engine.reproduce('/target', '/input',
+                          ['-some_arg=1', '-rss_limit_mb=4096'], 30)
+    self.runner.run_single_testcase.assert_called_once_with(
+        '/input',
+        timeout=30,
+        additional_args=['-some_arg=1', '-rss_limit_mb=4096', '-runs=100'])
+
+  def test_minimize_testcase_defaults_rss_limit(self):
+    """Test that minimize_testcase defaults rss_limit_mb when missing."""
+    self.runner.minimize_crash.return_value = new_process.ProcessResult(
+        command=['/target'], return_code=0, output='', time_executed=1)
+    self.engine.minimize_testcase('/target', [], '/input', '/output', 30)
+    self.runner.minimize_crash.assert_called_once()
+    _, kwargs = self.runner.minimize_crash.call_args
+    self.assertIn('-rss_limit_mb=0', kwargs['additional_args'])
+
+  def test_cleanse_defaults_rss_limit(self):
+    """Test that cleanse defaults rss_limit_mb when missing."""
+    self.runner.cleanse_crash.return_value = new_process.ProcessResult(
+        command=['/target'], return_code=0, output='', time_executed=1)
+    self.engine.cleanse('/target', [], '/input', '/output', 30)
+    self.runner.cleanse_crash.assert_called_once()
+    _, kwargs = self.runner.cleanse_crash.call_args
+    self.assertIn('-rss_limit_mb=0', kwargs['additional_args'])
+
+  def test_unparseable_arguments_are_passed_through(self):
+    """Test that arguments which cannot be parsed are left untouched.
+
+    Uploaded testcases can have arbitrary arguments that do not match
+    FuzzerArguments' parsing regex. Running without rss_limit_mb is preferable
+    to failing the task.
+    """
+    self.runner.minimize_crash.return_value = new_process.ProcessResult(
+        command=['/target'], return_code=0, output='', time_executed=1)
+    self.engine.minimize_testcase('/target', ['--disable-logging'], '/input',
+                                  '/output', 30)
+    self.runner.minimize_crash.assert_called_once()
+    _, kwargs = self.runner.minimize_crash.call_args
+    self.assertEqual(['--disable-logging'], kwargs['additional_args'])
+
+
 class BaseIntegrationTest(unittest.TestCase):
   """Base integration tests."""