Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Wed, 25 Mar 2020 18:35:22 +0100
From:   Jessica Yu <jeyu@kernel.org>
To:     Peter Zijlstra <peterz@infradead.org>
Cc:     x86@kernel.org, linux-kernel@vger.kernel.org, rostedt@goodmis.org,
        mhiramat@kernel.org, bristot@redhat.com, jbaron@akamai.com,
        torvalds@linux-foundation.org, tglx@linutronix.de,
        mingo@kernel.org, namit@vmware.com, hpa@zytor.com, luto@kernel.org,
        ard.biesheuvel@linaro.org, jpoimboe@redhat.com
Subject: Re: [RESEND][PATCH v3 03/17] module: Properly propagate
 MODULE_STATE_COMING failure
Message-ID: <20200325173519.GA5415@linux-8ccs>
References: <20200324135603.483964896@infradead.org>
 <20200324142245.445253190@infradead.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Disposition: inline
In-Reply-To: <20200324142245.445253190@infradead.org>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

+++ Peter Zijlstra [24/03/20 14:56 +0100]:
>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: jeyu@kernel.org
>---
> 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;
> }
>
> static int unknown_module_param_cb(char *param, char *val, const char *modname,
>

This looks fine to me - klp_module_going() is only called after
successful klp_module_coming(), and klp_module_going() is fine with
mod->state still being MODULE_STATE_COMING here. Would be good to have
livepatch folks double check. Which reminds me - Miroslav had pointed
out in the past that if there is an error when calling the COMING
notifiers, the GOING notifiers will be called while the mod->state is
still MODULE_STATE_COMING. I've briefly looked through all the module
notifiers and it looks like nobody is looking at mod->state directly
at least.

Acked-by: Jessica Yu <jeyu@kernel.org>