linux-kernel - Re: [RFC PATCH 2/9] livepatch: separate enabled and patched states

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]

Date:	Fri, 13 Feb 2015 13:57:38 +0100 (CET)
From:	Miroslav Benes <mbenes@...e.cz>
To:	Josh Poimboeuf <jpoimboe@...hat.com>
cc:	Seth Jennings <sjenning@...hat.com>, Jiri Kosina <jkosina@...e.cz>,
	Vojtech Pavlik <vojtech@...e.cz>,
	Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>,
	live-patching@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH 2/9] livepatch: separate enabled and patched states

On Mon, 9 Feb 2015, Josh Poimboeuf wrote:

> Once we have a consistency model, patches and their objects will be
> enabled and disabled at different times.  For example, when a patch is
> disabled, its loaded objects' funcs can remain registered with ftrace
> indefinitely until the unpatching operation is complete and they're no
> longer in use.
> 
> It's less confusing if we give them different names: patches can be
> enabled or disabled; objects (and their funcs) can be patched or
> unpatched:
> 
> - Enabled means that a patch is logically enabled (but not necessarily
>   fully applied).
> 
> - Patched means that an object's funcs are registered with ftrace and
>   added to the klp_ops func stack.
> 
> Also, since these states are binary, represent them with boolean-type
> variables instead of enums.

They are binary now but will it hold also in the future? I cannot come up 
with any other possible state of the function right now, but that doesn't 
mean there isn't any. It would be sad to return it back to enums one day 
:)

Also would it be useful to expose patched variable for functions and 
objects in sysfs?

Two small things below...

> Signed-off-by: Josh Poimboeuf <jpoimboe@...hat.com>
> ---
>  include/linux/livepatch.h | 15 ++++-----
>  kernel/livepatch/core.c   | 79 +++++++++++++++++++++++------------------------
>  2 files changed, 45 insertions(+), 49 deletions(-)
> 
> diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
> index 95023fd..22a67d1 100644
> --- a/include/linux/livepatch.h
> +++ b/include/linux/livepatch.h
> @@ -28,11 +28,6 @@
>  
>  #include <asm/livepatch.h>
>  
> -enum klp_state {
> -	KLP_DISABLED,
> -	KLP_ENABLED
> -};
> -
>  /**
>   * struct klp_func - function structure for live patching
>   * @old_name:	name of the function to be patched
> @@ -42,6 +37,7 @@ enum klp_state {
>   * @kobj:	kobject for sysfs resources
>   * @state:	tracks function-level patch application state
>   * @stack_node:	list node for klp_ops func_stack list
> + * @patched:	the func has been added to the klp_ops list
>   */
>  struct klp_func {
>  	/* external */
> @@ -59,8 +55,8 @@ struct klp_func {
>  
>  	/* internal */
>  	struct kobject kobj;
> -	enum klp_state state;
>  	struct list_head stack_node;
> +	int patched;
>  };

@state remains in the comment above

>  /**
> @@ -90,7 +86,7 @@ struct klp_reloc {
>   * @kobj:	kobject for sysfs resources
>   * @mod:	kernel module associated with the patched object
>   * 		(NULL for vmlinux)
> - * @state:	tracks object-level patch application state
> + * @patched:	the object's funcs have been add to the klp_ops list
>   */
>  struct klp_object {
>  	/* external */
> @@ -101,7 +97,7 @@ struct klp_object {
>  	/* internal */
>  	struct kobject *kobj;
>  	struct module *mod;
> -	enum klp_state state;
> +	int patched;
>  };
>  
>  /**
> @@ -111,6 +107,7 @@ struct klp_object {
>   * @list:	list node for global list of registered patches
>   * @kobj:	kobject for sysfs resources
>   * @state:	tracks patch-level application state
> + * @enabled:	the patch is enabled (but operation may be incomplete)
>   */
>  struct klp_patch {
>  	/* external */
> @@ -120,7 +117,7 @@ struct klp_patch {
>  	/* internal */
>  	struct list_head list;
>  	struct kobject kobj;
> -	enum klp_state state;
> +	int enabled;
>  };

Dtto

Miroslav
--
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/