sof icon indicating copy to clipboard operation
sof copied to clipboard

General lack of .config control exposed by fuzzer .config divergence

Open marc-hb opened this issue 1 year ago • 6 comments

Originally posted by @marc-hb in https://github.com/thesofproject/sof/issues/9343#issuecomment-2278389786

Even if #9356 is perfect, it does not change anything to the apparently messy Kconfig situation... @andyross could you help there? Should CONFIG_INCOHERENT really be gated by CONFIG_XTENSA? Can we align build-fuzz/.config with build/.config more? Some differences are unavoidable but they should be as small as possible: fuzzing something different from the product is both dangerous and a bit of a waste.


Let's compare the default build-mtl/zephyr/.config with build-fuzz/zephyr/.config as of today's SOF commit 1009ba774be2

./scripts/xtensa-build-zephyr.py -p mtl
./scripts/fuzz.sh -p -b -- -DCONFIG_IPC_MAJOR_4=y
diff  ../build-mtl/zephyr/.config build-fuzz/zephyr/.config | diffstat
 unknown |  617 ++++++++++++++++++++++++++++++++++++++++++++++++----------------------------------------------------------------------------------------------------------------
 1 file changed, 186 insertions(+), 431 deletions(-)

That's a lot. Some of these differences are unavoidable because they are "hardware"-dependent, for instance: CONFIG_DT_HAS_CDNS_TENSILICA_XTENSA_LX7_ENABLED or CONFIG_ZEPHYR_POSIX_FUZZ_IRQ. Other diffferences are wrong because they are "pure software" like CONFIG_AMS or CONFIG_SOF_TELEMETRY. Can these last two software features affect fuzzing? Maybe, maybe not: we don't want to know. Pure software differences should be as small as possible, period. That is: if we want to fuzz and find security issues in code used in production.

So where do CONFIG_AMS and CONFIG_SOF_TELEMETRY come from?

sof]$ git grep CONFIG_AMS
app/boards/intel_adsp_ace15_mtpm.conf:CONFIG_AMS=y
app/boards/intel_adsp_ace20_lnl.conf:CONFIG_AMS=y
app/boards/intel_adsp_cavs25.conf:CONFIG_AMS=y

Enjoy the typical duplication.

As a quick fix, maybe we can bring the fuzzed .config closer to the real thing with this simple addition?

./scripts/fuzz.sh -p -b -- -DCONFIG_IPC_MAJOR_4=y -DEXTRA_CONF_FILE=boards/intel_adsp_ace15_mtpm.conf

Unfortunately not. First we get a gazillion of "hardware" warnings:

warning: POWER_DOMAIN_INTEL_ADSP (defined at drivers/power_domain/Kconfig:40) was assigned the value
'y' but got the value 'n'. Check these unsatisfied dependencies:
DT_HAS_INTEL_ADSP_POWER_DOMAIN_ENABLED (=n). 

warning: DEBUG_COREDUMP (defined at subsys/debug/coredump/Kconfig:4) was assigned the value 'y' but
got the value 'n'. Check these unsatisfied dependencies: ...

Then:

CMake Error at /var/home/mherber2/SOF/zephyr/cmake/modules/extensions.cmake:5340 (message):
  add_llext_target: only one source file is supported for ELF object file

cc:

  • #9222
  • https://docs.zephyrproject.org/latest/build/snippets/writing.html
  • https://github.com/zephyrproject-rtos/zephyr/issues/78978

marc-hb avatar Aug 22 '24 00:08 marc-hb

maybe we can bring the fuzzed .config closer to the real thing with this simple boards/intel_adsp_ace15_mtpm.conf addition? Unfortunately not.

That's because boards/intel_adsp_ace15_mtpm.conf is a "grab bag" of items totally unrelated to each other, including some actually ace15-specific stuff mixed up with pure software configuration like CONFIG_AMS. The latter being naturally duplicated all over the place.

The fix is to study https://docs.zephyrproject.org/latest/build/kconfig/setting.html#initial-conf and implement some proper "overlays" where (at least) hardware and software are separated.

Note how this comment is totally unrelated to fuzzing; fuzzing is just the messenger here.

marc-hb avatar Aug 22 '24 00:08 marc-hb

@marc-hb can this be closed now with #9389 merged ?

lgirdwood avatar Aug 23 '24 13:08 lgirdwood

No, there are .config issues pretty much everywhere we look. Examples:

  • #9404
  • #9405
  • https://github.com/thesofproject/sof/pull/9151#pullrequestreview-2258270815

I will also submit some changes to reduce a bit the gap between build-fuzz/zephyr/.config and products.

marc-hb avatar Aug 24 '24 01:08 marc-hb

I will also submit some changes to reduce a bit the gap between build-fuzz/zephyr/.config and products.

Done:

  • #9409

There are probably more CONFIG_ that should be added (and cleaned up). #9409 sets the stage/adds the "framework" showing how to do that.

marc-hb avatar Aug 27 '24 01:08 marc-hb

The CONFIG_ gap between fuzzing and production is still too big and there are still weird things in the production config (like b2f79a0c3df855) but I don't have any time left for this.

marc-hb avatar Sep 04 '24 23:09 marc-hb

Another problem noticed: commit 87e973d32f4e "downgrades" m to n instead of y. Maybe CONFIG_LLEXT=n would be better instead?

marc-hb avatar Sep 18 '24 17:09 marc-hb

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

github-actions[bot] avatar Jun 19 '25 03:06 github-actions[bot]