module notifier: was Re: [PATCH 2/2] kernel: add support for live patching

[Petr Mladek <pmladek@suse.cz>]

On Thu 2014-11-06 08:39:08, Seth Jennings wrote:
> This commit introduces code for the live patching core.  It implements
> an ftrace-based mechanism and kernel interface for doing live patching
> of kernel and kernel module functions.
> 
> It represents the greatest common functionality set between kpatch and
> kgraft and can accept patches built using either method.
> 
> This first version does not implement any consistency mechanism that
> ensures that old and new code do not run together.  In practice, ~90% of
> CVEs are safe to apply in this way, since they simply add a conditional
> check.  However, any function change that can not execute safely with
> the old version of the function can _not_ be safely applied in this
> version.

[...]
 
> +/******************************
> + * module notifier
> + *****************************/
> +
> +static int lp_module_notify(struct notifier_block *nb, unsigned long action,
> +			    void *data)
> +{
> +	struct module *mod = data;
> +	struct lpc_patch *patch;
> +	struct lpc_object *obj;
> +	int ret = 0;
> +
> +	if (action != MODULE_STATE_COMING)
> +		return 0;

IMHO, we should handle also MODULE_STATE_GOING. We should unregister
the ftrace handlers and update the state of the affected objects
(ENABLED -> DISABLED)

> +	down(&lpc_mutex);
> +
> +	list_for_each_entry(patch, &lpc_patches, list) {
> +		if (patch->state == DISABLED)
> +			continue;
> +		list_for_each_entry(obj, &patch->objs, list) {
> +			if (strcmp(obj->name, mod->name))
> +				continue;
> +			pr_notice("load of module '%s' detected, applying patch '%s'\n",
> +				  mod->name, patch->mod->name);
> +			obj->mod = mod;
> +			ret = lpc_enable_object(patch->mod, obj);
> +			if (ret)
> +				goto out;
> +			break;
> +		}
> +	}
> +
> +	up(&lpc_mutex);
> +	return 0;
> +out:

I would name this err_our or so to make it clear that it is used when
something fails.

> +	up(&lpc_mutex);
> +	WARN("failed to apply patch '%s' to module '%s'\n",
> +		patch->mod->name, mod->name);
> +	return 0;
> +}
> +
> +static struct notifier_block lp_module_nb = {
> +	.notifier_call = lp_module_notify,
> +	.priority = INT_MIN, /* called last */

The handler for MODULE_STATE_COMMING would need have higger priority,
if we want to cleanly unregister the ftrace handlers.

Best Regards,
Petr