lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Mon, 24 Jun 2019 11:52:13 -0400 From: Joel Fernandes <joel@...lfernandes.org> To: Mathieu Desnoyers <mathieu.desnoyers@...icios.com> Cc: Peter Zijlstra <peterz@...radead.org>, "Frank Ch. Eigler" <fche@...hat.com>, Jessica Yu <jeyu@...nel.org>, linux-kernel <linux-kernel@...r.kernel.org>, Josh Poimboeuf <jpoimboe@...hat.com>, jikos@...nel.org, mbenes@...e.cz, Petr Mladek <pmladek@...e.com>, Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>, Andrew Morton <akpm@...ux-foundation.org>, Robert Richter <rric@...nel.org>, rostedt <rostedt@...dmis.org>, Ingo Molnar <mingo@...hat.com>, Martin KaFai Lau <kafai@...com>, Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>, paulmck <paulmck@...ux.ibm.com>, Ard Biesheuvel <ard.biesheuvel@...aro.org>, Thomas Gleixner <tglx@...utronix.de>, oprofile-list@...ts.sf.net, netdev <netdev@...r.kernel.org>, bpf@...r.kernel.org Subject: Re: [PATCH 2/3] module: Fix up module_notifier return values. On Mon, Jun 24, 2019 at 10:01:04AM -0400, Mathieu Desnoyers wrote: > ----- On Jun 24, 2019, at 5:18 AM, Peter Zijlstra peterz@...radead.org wrote: > > > While auditing all module notifiers I noticed a whole bunch of fail > > wrt the return value. Notifiers have a 'special' return semantics. > > > > Cc: Robert Richter <rric@...nel.org> > > Cc: Steven Rostedt <rostedt@...dmis.org> > > Cc: Ingo Molnar <mingo@...hat.com> > > Cc: Alexei Starovoitov <ast@...nel.org> > > Cc: Daniel Borkmann <daniel@...earbox.net> > > Cc: Martin KaFai Lau <kafai@...com> > > Cc: Song Liu <songliubraving@...com> > > Cc: Yonghong Song <yhs@...com> > > Cc: Mathieu Desnoyers <mathieu.desnoyers@...icios.com> > > Cc: "Paul E. McKenney" <paulmck@...ux.ibm.com> > > Cc: "Joel Fernandes (Google)" <joel@...lfernandes.org> > > Cc: Ard Biesheuvel <ard.biesheuvel@...aro.org> > > Cc: Thomas Gleixner <tglx@...utronix.de> > > Cc: oprofile-list@...ts.sf.net > > Cc: linux-kernel@...r.kernel.org > > Cc: netdev@...r.kernel.org > > Cc: bpf@...r.kernel.org > > Signed-off-by: Peter Zijlstra (Intel) <peterz@...radead.org> > > Thanks Peter for looking into this, especially considering your > endless love for kernel modules! ;) > > It's not directly related to your changes, but I notice that > kernel/trace/trace_printk.c:hold_module_trace_bprintk_format() > appears to leak memory. Am I missing something ? Could you elaborate? Do you mean there is no MODULE_STATE_GOING notifier check? If that's what you mean then I agree, there should be some place where the format structures are freed when the module is unloaded no? > > With respect to your changes: > Reviewed-by: Mathieu Desnoyers <mathieu.desnoyers@...icios.com> Looks good to me too. Reviewed-by: Joel Fernandes (Google) <joel@...lfernandes.org> Could we CC stable so that the fix is propagated to older kernels? thanks, - Joel > I have a similar erroneous module notifier return value pattern > in lttng-modules as well. I'll go fix it right away. CCing > Frank Eigler from SystemTAP which AFAIK use a copy of > lttng-tracepoint.c in their project, which should be fixed > as well. I'm pasting the lttng-modules fix below. > > Thanks! > > Mathieu > > -- > > commit 5eac9d146a7d947f0f314c4f7103c92cbccaeaf3 > Author: Mathieu Desnoyers <mathieu.desnoyers@...icios.com> > Date: Mon Jun 24 09:43:45 2019 -0400 > > Fix: lttng-tracepoint module notifier should return NOTIFY_OK > > Module notifiers should return NOTIFY_OK on success rather than the > value 0. The return value 0 does not seem to have any ill side-effects > in the notifier chain caller, but it is preferable to respect the API > requirements in case this changes in the future. > > Notifiers can encapsulate a negative errno value with > notifier_from_errno(), but this is not needed by the LTTng tracepoint > notifier. > > The approach taken in this notifier is to just print a console warning > on error, because tracing failure should not prevent loading a module. > So we definitely do not want to stop notifier iteration. Returning > an error without stopping iteration is not really that useful, because > only the return value of the last callback is returned to notifier chain > caller. > > Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@...icios.com> > > diff --git a/lttng-tracepoint.c b/lttng-tracepoint.c > index bbb2c7a4..8298b397 100644 > --- a/lttng-tracepoint.c > +++ b/lttng-tracepoint.c > @@ -256,7 +256,7 @@ int lttng_tracepoint_coming(struct tp_module *tp_mod) > } > } > mutex_unlock(<tng_tracepoint_mutex); > - return 0; > + return NOTIFY_OK; > } > > static > > > -- > Mathieu Desnoyers > EfficiOS Inc. > http://www.efficios.com
Powered by blists - more mailing lists