On Fri, Mar 11, 2022 at 05:00:32PM -0500, Paul Moore wrote: > On Fri, Mar 11, 2022 at 12:47 PM Mickaël Salaün <mic@xxxxxxxxxxx> wrote: > > From: Mickaël Salaün <mic@xxxxxxxxxxxxxxxxxxx> > > > > Replace panic() calls from device_initcall(blacklist_init) with proper > > error handling using -ENODEV. > > > > Suggested-by: Jarkko Sakkinen <jarkko@xxxxxxxxxx> [1] > > Link: https://lore.kernel.org/r/Yik0C2t7G272YZ73@xxxxxx [1] > > Signed-off-by: Mickaël Salaün <mic@xxxxxxxxxxxxxxxxxxx> > > Link: https://lore.kernel.org/r/20220311174741.250424-2-mic@xxxxxxxxxxx > > --- > > certs/blacklist.c | 27 +++++++++++++++++++++------ > > 1 file changed, 21 insertions(+), 6 deletions(-) > > I'm not sure we can safely rely on a non-zero error code saving us in > the care of failure, can we? > > The blacklist_init() function is registered as an initcall via > device_initcall() which I believe is either executed via > do_init_module() in the case of a dynamic module load, or via > do_initcalls() if built into the kernel. In either case the result is > that the module/functionality doesn't load and the kernel continues on > executing. While this could be acceptable for some non-critical > modules, if this particular module fails to load it defeats the > certificate/key based deny list for signed modules, yes? > > I completely understand the strong desire to purge the kernel of > panic()s, BUG()s, and the like, but if a critical piece of security > functionality that users expect to be present fails to initialize, > panic()ing is likely the right thing to do. OK, I get this. What this function should have is this information documented in the header. Otherwise, this is just confusing. BR, Jarkko
