Hello,
Following up on our previous discussion in this thread, I've posted a patch to disable getc() by default:
https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/23549
The aim is not only to disable getc() - as in, not being able to call it anymore - but also to remove all getc-related code from the firmware binaries so that there is no remnant getc code which could be used as a gadget as part of a bigger security attack.
Let me explain the changes introduced by this patch and the design choices I have made - which are all up for discussion!
Changes =======
1. By default, getc() is now compiled out at the multi-console framework level.
This means it is no longer possible to invoke any of the console drivers's getc() callbacks through the multi-console framework.
Note that TF-A libc does not provide getc() so console_getc() is the highest-level API we've got in this area.
2. By default, if a console driver attempts to register a getc() callback into the multi-console framework, TF-A now fails to build.
For example with the Arm PL011 console driver: drivers/arm/pl011/aarch64/pl011_console.S: Assembler messages: drivers/arm/pl011/aarch64/pl011_console.S:110: Error: getc() support is disabled. Do not register a getc() console callback.
3. By default, all console drivers which registered a getc() callback no longer do.
All of these changes can be reverted by building TF-A with ENABLE_CONSOLE_GETC=1.
Code Pruning ============
I believe that the combination of the aforementioned 3 changes eliminate all getc-related code from the firmware binaries. Individual console drivers should not need to put their console_xxx_getc() definitions behind an '#if ENABLE_CONSOLE_GETC' guard for them to be compiled out, so long as the -ffunction-sections / --gc-sections flags combo is used, which is standard in TF-A. I've verified that with a couple of platforms / drivers. With this flags combo, console_xxx_getc() function code is isolated in its own section, which gets garbage-collected at link time since there is no reference to it.
The above assumes that platform code itself does not explicitly call console_xxx_getc(). If that's the case, then console_xxx_getc() code would obviously be retained in the binary.
Alternative Designs ===================
As an alternative to 1., I considered leaving console_getc() enabled but change the implementation to unconditionally return EOF. This would have achieved the same outcome - not being able to call console drivers's getc() callbacks anymore. However, I am not sure this is the desired behaviour for all platforms. Besides, I imagine such a behavioural change could go unnoticed (because getc() returning EOF is not an error case) and thus trigger annoying bugs in developers' setups. So I gave up on this idea.
As an alternative to 2., I considered handling this error at runtime, at the console driver's registration time. In other words, adding a test in console_register() function to make sure that the console driver registers no getc() callback (i.e. a NULL pointer). However, this incurs a slight performance penalty which we can avoid. More importantly, this requires all callers of console_register() to check its return code, which is far from true today!
Both of these issues could have been addressed with a debug assertion (no performance penalty in release builds - which are free of debug assertions - and no need to check the return code of console_register()) but printing the assertion message requires a functional console, which is not available at this time since we are still in the process of setting it up!
Please let me know if you have any concerns with this change or can think of better ways to implement it.
Best regards, Sandrine