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

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20141107171307.GC1136@dhcp128.suse.cz>
Date:	Fri, 7 Nov 2014 18:13:07 +0100
From:	Petr Mladek <pmladek@...e.cz>
To:	Seth Jennings <sjenning@...hat.com>
Cc:	Josh Poimboeuf <jpoimboe@...hat.com>,
	Jiri Kosina <jkosina@...e.cz>,
	Vojtech Pavlik <vojtech@...e.cz>,
	Steven Rostedt <rostedt@...dmis.org>,
	live-patching@...r.kernel.org, kpatch@...hat.com,
	linux-kernel@...r.kernel.org
Subject: module notifier: was Re: [PATCH 2/2] kernel: add support for live
 patching

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/