Messages in this thread | | | Date | Tue, 8 Oct 2019 15:08:57 +0200 (CEST) | From | Miroslav Benes <> | Subject | Re: [PATCH v2 3/4] module: Properly propagate MODULE_STATE_COMING failure |
| |
On Mon, 7 Oct 2019, Peter Zijlstra wrote:
> Now that notifiers got unbroken; use the proper interface to handle > notifier errors and propagate them. > > There were already MODULE_STATE_COMING notifiers that failed; notably: > > - jump_label_module_notifier() > - tracepoint_module_notify() > - bpf_event_notify() > > By propagating this error, we fix those users. > > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> > Cc: Yonghong Song <yhs@fb.com> > Cc: Alexei Starovoitov <ast@kernel.org> > Cc: Daniel Borkmann <daniel@iogearbox.net> > Cc: Song Liu <songliubraving@fb.com> > Cc: Jessica Yu <jeyu@kernel.org> > Cc: Martin KaFai Lau <kafai@fb.com> > --- > kernel/module.c | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > --- a/kernel/module.c > +++ b/kernel/module.c > @@ -3751,9 +3751,13 @@ static int prepare_coming_module(struct > if (err) > return err; > > - blocking_notifier_call_chain(&module_notify_list, > - MODULE_STATE_COMING, mod); > - return 0; > + err = blocking_notifier_call_chain_robust(&module_notify_list, > + MODULE_STATE_COMING, MODULE_STATE_GOING, mod); > + err = notifier_to_errno(err); > + if (err) > + klp_module_going(mod); > + > + return err; > }
It looks almost ok. At least klp_ error handling is correct now. I see only one possible problem. If there is an error in a MODULE_STATE_COMING notifier, all MODULE_STATE_GOING notifiers will be called with mod->state set to MODULE_STATE_COMING. Not nice. I don't think it is actually a problem, because all notifiers, that I checked, only use the correct value (MODULE_STATE_COMING or MODULE_STATE_GOING, coming from the function parameter) and not mod->state. Better to doublecheck though.
However, mod->state is not set to MODULE_STATE_GOING anywhere under bug_cleanup label in load_module(). That is a bug and it is there regardless of this patch.
Jessica?
Regards Miroslav
| |