Messages in this thread | | | Date | Tue, 9 Jan 2024 11:24:47 -0800 | From | Josh Poimboeuf <> | Subject | Re: [PATCH v2 1/3] objtool: Make objtool check actually fatal upon fatal errors |
| |
On Mon, Jan 08, 2024 at 10:15:34AM +0100, Ingo Molnar wrote: > > * Dimitri John Ledkov <dimitri.ledkov@canonical.com> wrote: > > > Currently function calls within check() are sensitive to fatal errors > > (negative return codes) and abort execution prematurely. However, in > > all such cases the check() function still returns 0, and thus > > resulting in a successful kernel build. > > > > The only correct code paths were the ones that escpae the control flow > > with `return ret`. > > > > Make the check() function return `ret` status code, and make all > > negative return codes goto that instruction. This makes fatal errors > > (not warnings) from various function calls actually fail the > > build. E.g. if create_retpoline_sites_sections() fails to create elf > > section pair retpoline_sites the tool now exits with an error code. > > > > Signed-off-by: Dimitri John Ledkov <dimitri.ledkov@canonical.com> > > So, is this not expected to be the case anymore: > > > out: > > - /* > > - * For now, don't fail the kernel build on fatal warnings. These > > - * errors are still fairly common due to the growing matrix of > > - * supported toolchains and their recent pace of change. > > - */ > > - return 0; > > ? > > How about making it only fatal if CONFIG_WERROR=y, ie. an analogue to our > treatment of compiler warnings?
Objtool has two classes of warnings:
1) "fatal"
- allocation failures - CFG recreation failures - annotation parsing errors - other objtool bugs
2) non-"fatal":
- missing security features (retpolines, IBT, SLS INT3) - unreachable instructions (note this warning may indicate more serious issues like an incomplete or buggy objtool CFG)
The first class of "warning" is actually an error. It means objtool couldn't reasonably continue, so it exited early. I'm thinking this should always fail the build so it can be reported and fixed ASAP.
We tried doing that before, but ending up reverting it (for raisins). We should try again (as per the above patch).
The second class of warning, though it doesn't abort objtool, can still be quite serious. Ignoring it can fail the boot, or can expose the user to certain attacks.
My proposal would be to always fail for #1, and to make #2 dependent on CONFIG_WERROR.
Note the latter may be problematic at the moment due to some outstanding warnings reported by Arnd and randconfig build bots. I try to fix those when I can, but any help would be appreciated.
-- Josh
| |