Build an external-data FIT, then point one image's data-offset far past the end of the file and, separately, make its data-size run a few bytes past the end, re-processing the result with mkimage -F each time. Both must be rejected with a diagnostic naming that image. Previously the first crashed mkimage and the second silently imported the bytes beyond the end of the file as image data.
Also check that a FIT whose images are all empty, which mkimage -E -p writes with data-position past the end of the file, still re-processes, since an empty region has nothing to read. Finally, now that the import reads the properties through the FIT accessors, check that an empty data-offset or data-size is skipped like a missing property, so that the hashing stage reports the image instead of the import reading it from the bytes that follow it. Signed-off-by: Aristo Chen <[email protected]> --- test/py/tests/test_fit_import_data.py | 91 ++++++++++++++++++++++++++- 1 file changed, 90 insertions(+), 1 deletion(-) diff --git a/test/py/tests/test_fit_import_data.py b/test/py/tests/test_fit_import_data.py index 95f7114243e..c6f03db17ad 100644 --- a/test/py/tests/test_fit_import_data.py +++ b/test/py/tests/test_fit_import_data.py @@ -3,7 +3,7 @@ # # Test mkimage import of external data in fit_import_data() -"""Regression test for stale per-image state in fit_import_data(). +"""Regression tests for fit_import_data(). The import loop used to keep the data pointer and external property name of the previous image, so an image node carrying data-size but neither @@ -11,6 +11,11 @@ data-offset nor data-position imported the previous image's data and then made mkimage abort without printing any diagnostic. Such a node must be skipped by the import and reported by the later processing stages instead. + +The import also read each image's external data at the offset and size +the FIT declares without checking them against the file, so a region +reaching past the end of the file crashed mkimage or imported unrelated +memory as image data. Such a region must be rejected before it is read. """ import os @@ -113,3 +118,87 @@ def test_fit_import_data_missing_offset(ubman): capture_output=True, text=True) assert result.returncode != 0 assert "Can't get image data/size" in result.stderr + + [email protected]('sandbox') [email protected]('dtc') [email protected]('fdtput') +def test_fit_import_data_out_of_range(ubman): + """External data outside the file must be rejected, not read""" + mkimage = os.path.join(ubman.config.build_dir, 'tools/mkimage') + params = { + 'kernel1': fit_util.make_kernel(ubman, 'oob-kernel1.bin', 'first'), + 'kernel2': fit_util.make_kernel(ubman, 'oob-kernel2.bin', 'second'), + } + its = fit_util.make_its(ubman, BASE_ITS, params, 'oob.its') + itb = fit_util.make_fname(ubman, 'oob.itb') + + result = subprocess.run([mkimage, '-E', '-f', its, itb], + capture_output=True, text=True) + assert result.returncode == 0, result.stderr + + # Point kernel-2's data far past the end of the file + fdtput_keep_data(itb, '/images/kernel-2', 'data-offset', '0x40000000', + opts=['-t', 'i']) + result = subprocess.run([mkimage, '-F', itb], + capture_output=True, text=True) + assert result.returncode != 0 + assert "image 'kernel-2': external data" in result.stderr + assert 'outside the FIT file' in result.stderr + + # A size running just past the end of the file must be rejected too. + # kernel-2's data comes last, padded to 4 bytes, so 4 bytes more than + # its real size always reach past the end. + result = subprocess.run([mkimage, '-E', '-f', its, itb], + capture_output=True, text=True) + assert result.returncode == 0, result.stderr + size = os.path.getsize(params['kernel2']) + 4 + fdtput_keep_data(itb, '/images/kernel-2', 'data-size', f'{size:#x}', + opts=['-t', 'i']) + result = subprocess.run([mkimage, '-F', itb], + capture_output=True, text=True) + assert result.returncode != 0 + assert "image 'kernel-2': external data" in result.stderr + + # An empty region has nothing to read, so it is accepted even past the + # end of the file, which is where mkimage -E -p places it when every + # image is empty + for name in ('kernel1', 'kernel2'): + params[name] = fit_util.make_fname(ubman, f'oob-empty-{name}.bin') + with open(params[name], 'wb'): + pass + its = fit_util.make_its(ubman, BASE_ITS, params, 'oob-empty.its') + result = subprocess.run([mkimage, '-E', '-p', '0x1000', '-f', its, itb], + capture_output=True, text=True) + assert result.returncode == 0, result.stderr + assert os.path.getsize(itb) < 0x1000 + result = subprocess.run([mkimage, '-F', itb], + capture_output=True, text=True) + assert result.returncode == 0, result.stderr + + [email protected]('sandbox') [email protected]('dtc') [email protected]('fdtput') +def test_fit_import_data_short_prop(ubman): + """An external data property shorter than one cell must not be read""" + mkimage = os.path.join(ubman.config.build_dir, 'tools/mkimage') + params = { + 'kernel1': fit_util.make_kernel(ubman, 'short-kernel1.bin', 'first'), + 'kernel2': fit_util.make_kernel(ubman, 'short-kernel2.bin', 'second'), + } + its = fit_util.make_its(ubman, BASE_ITS, params, 'short.its') + itb = fit_util.make_fname(ubman, 'short.itb') + + # An empty property must be skipped like a missing one, so that the + # hashing stage reports the image, rather than read from the bytes + # that follow it, which would import a few bytes or the wrong data + for prop in ('data-offset', 'data-size'): + result = subprocess.run([mkimage, '-E', '-f', its, itb], + capture_output=True, text=True) + assert result.returncode == 0, result.stderr + fdtput_keep_data(itb, '/images/kernel-2', prop) + result = subprocess.run([mkimage, '-F', itb], + capture_output=True, text=True) + assert result.returncode != 0 + assert "Can't get image data/size" in result.stderr -- 2.43.0
