Hello,

Generally speaking, I think we should keep TF-A codebase free of dead code. Dead code can lead to confusion for developers and also can potentially be used as gadgets to exploit some security vulnerability.

Could you please provide a list of such unused functions?

Please bare in mind that some functions will only be used on some platforms or when a specific feature is enabled through a compile-time flag. So these would not be considered as dead code as such.

Best regards, Sandrine

Hi Nithin,

May I ask which platform you are basing your analysis on? And what are the build options you're using?

page_align() function is used in a number of files:

plat/arm/board/fvp/fvp_bl31_setup.c: hw_config_base_align = page_align(hw_config_info->config_addr, DOWN); plat/arm/board/fvp/fvp_bl31_setup.c: mapped_size_align = page_align(hw_config_info->config_max_size, UP); plat/arm/board/fvp/sp_min/fvp_sp_min_setup.c: hw_config_base_align = page_align(hw_config_info->config_addr, DOWN); plat/arm/board/fvp/sp_min/fvp_sp_min_setup.c: mapped_size_align = page_align(hw_config_info->config_max_size, UP); plat/common/plat_spmd_manifest.c: pm_base_align = page_align(pm_base, UP); plat/common/plat_spmd_manifest.c: pm_base_align = page_align(pm_base, DOWN); services/spd/opteed/opteed_main.c: mapped_data_pa = page_align(data_pa, DOWN); services/spd/opteed/opteed_main.c: data_map_size = page_align(data_size + (mapped_data_pa - data_pa), UP); services/spd/opteed/opteed_main.c: target_pa = page_align(image_pa, DOWN); services/spd/opteed/opteed_main.c: target_size = page_align(target_end_pa, UP) - target_pa; services/std_svc/drtm/drtm_main.c: dlme_data_min_size = page_align(dlme_data_min_size, UP)/PAGE_SIZE; services/std_svc/drtm/drtm_measurements.c: dlme_img_mapping_bytes = page_align(a->dlme_img_size, UP); services/std_svc/spm/el3_spmc/spmc_main.c: manifest_base_align = page_align(manifest_base, DOWN); services/std_svc/spmd/spmd_main.c: base_addr_align = page_align(base_addr, DOWN); services/std_svc/spmd/spmd_main.c: mapped_size_align = page_align(size, UP);

If you are building for FVP platform, it should be used by BL31. That being said, it's a small helper function, thus it might be inline, which would explain why it's not appearing in the symbol table. But it's definitely not dead code.

The load_*() family of functions are also used for sure, but only in BL1 and BL2 (BL31 does not load any images). So depending on which BL image you're looking at, this might be the reason why you're not seeing them in the symbol table.

Also, the variants with _auth_ in their names are ​only used if trusted boot is enabled (see TRUSTED_BOARD_BOOT build option in TF-A documentation). So again if you've not enabled this feature in the binary you're analysing, it would not be pulled.

Coming back to your question: These functions are not appearing in symbol table also, then no need to take actions on violations reported in these functions? I don't think we can make that claim generally. The compiler and linker options used in TF-A build system will try to remove any unused function. Thus, true dead code would not appear in the symbol table. But as I said, there are other reasons why a function might not appear in the symbol table : it could be inline ; or it could be unused on this specific platform / using these specific build options, but used in other build environments. In the previous 2 examples, this is not dead code.

Best regards, Sandrine

________________________________ From: Nithin G via TF-A tf-a@lists.trustedfirmware.org Sent: 31 August 2023 12:18 To: tf-a@lists.trustedfirmware.org tf-a@lists.trustedfirmware.org Subject: [TF-A] Re: MISRA-C violation for unused functions in Trusted Firmware-A(TFA) code

Hi Sandrine,

Thanks for responding,

Here few of the unused function present in bl_common.c file, 1.page_align 2.load_image 3.load_auth_image_internal 3.load_auth_image These functions are not appearing in symbol table also, then no need to take actions on violations reported in these functions?

Similar to these unused functions are present in other files also. shall we take action on violations reported in these, please suggest.

Regards, Nithin G -- TF-A mailing list -- tf-a@lists.trustedfirmware.org To unsubscribe send an email to tf-a-leave@lists.trustedfirmware.org

Hi Nithin,

On 9/5/23 07:56, Nithin G via TF-A wrote:

Hi Sandrine,

Sorry I was in vacation. Thanks for responding,

No problem.

we are using zynqmp platform and building using coverity build. consider for only our build files while building it generates, these functions are not called in any other files and also instead of bl1 and bl2 we are using FSBL (first stage bootloader).

OK so that explains why you're not seeing any of the BL1/BL2 specific functions being referenced.

If I take another example in bl31 files, multi_console.c file: 1.console_getc

I think you're right about this one. There does not seem to be any platform port calling console_getc() right now.

Thinking further about it, it's not surprising... TF-A writes lots of information to the console but getting input from the user is very unusual. Unlike some other firmwares (e.g. UEFI), TF-A does not implement a shell or any kind of interactive user input mechanism.

I guess getc() was implemented for completeness of the console drivers - once you've got all the console setup and putc(), getc() almost comes for free.

2.console_unregister

This is used on Arm, Broadcom and Marvell platforms. Hence, it's no dead code from the point of view of TF-A codebase, even though it's not used on zynqmp platforms.

You can easily see this using `git grep console_unregister` command from TF-A top directory:

$ git grep console_unregister drivers/console/multi_console.c:console_t *console_unregister(console_t *to_be_deleted) include/drivers/console.h:console_t *console_unregister(console_t *console); plat/arm/board/fvp/fvp_console.c: (void)console_unregister(&fvp_runtime_console); plat/arm/common/arm_console.c: (void)console_unregister(&arm_boot_console); plat/brcm/board/common/bcm_console.c: (void)console_unregister(&bcm_boot_console); plat/brcm/board/common/bcm_console.c: (void)console_unregister(&bcm_runtime_console); plat/marvell/armada/a8k/a80x0_puzzle/board/system_power.c: (void)console_unregister(&console); plat/marvell/armada/common/marvell_console.c: (void)console_unregister(&marvell_boot_console); plat/marvell/armada/common/marvell_console.c: (void)console_unregister(&marvell_runtime_console);

I suggest you cross-check Coverity results against `git grep` results to identify dead code.

interrupt_mgmt.c file: 1.disable_intr_rm_local

Used by TSPD and TLKD.

2.enable_intr_rm_local

Used by TSPD.

For the above functions also not called in other files if I consider our build files for zynqmp build. Like these I am getting many unused functions in other files. Please suggest?

Sorry, I am not sure what you're asking for... We obviously can't remove functions which are unused on zynqmp platforms if they are used by some other platforms in the tree. If you find true dead code (like console_getc() above) we can open a discussion on this. But otherwise, I can't offer any advice. I guess you need to find a way to tell Coverity what's true dead code and what can be safely ignored in the case of your platform...

Best regards, Sandrine