Live-Patching Archive on lore.kernel.org
 help / color / Atom feed
From: Miroslav Benes <mbenes@suse.cz>
To: Petr Mladek <pmladek@suse.com>
Cc: Jiri Kosina <jikos@kernel.org>,
	Josh Poimboeuf <jpoimboe@redhat.com>,
	Joe Lawrence <joe.lawrence@redhat.com>,
	Kamalesh Babulal <kamalesh@linux.vnet.ibm.com>,
	Nicolai Stange <nstange@suse.de>,
	live-patching@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 2/5] livepatch: Basic API to track system state changes
Date: Tue, 13 Aug 2019 15:33:28 +0200 (CEST)
Message-ID: <alpine.LSU.2.21.1908131525480.10477@pobox.suse.cz> (raw)
In-Reply-To: <20190719074034.29761-3-pmladek@suse.com>

On Fri, 19 Jul 2019, Petr Mladek wrote:

> This is another step how to help maintaining more livepatches.
> 
> One big help was the atomic replace and cumulative livepatches. These
> livepatches replace the already installed ones. Therefore it should
> be enough when each cumulative livepatch is consistent.
> 
> The problems might come with shadow variables and callbacks. They might
> change the system behavior or state so that it is no longer safe to
> go back and use an older livepatch or the original kernel code. Also
> any new livepatch must be able to detect what changes have already been
> done by the already installed livepatches.

"Also, a new livepatch must be able to detect changes which were made 
by the already installed livepatches." would sound better to me.
 
> This is where the livepatch system state tracking gets useful. It
> allows to:
> 
>   - find whether a system state has already been modified by
>     previous livepatches
> 
>   - store data needed to manipulate and restore the system state
> 
> The information about the manipulated system states is stored in an
> array of struct klp_state. It can be searched by two new functions
> klp_get_state() and klp_get_prev_state().
> 
> The dependencies are going to be solved by a version field added later.
> The only important information is that it will be allowed to modify
> the same state by more non-cumulative livepatches. It is the same logic
> as that it is allowed to modify the same function several times.

Wouldn't something like "It is similar to allowing to modify the same 
function several times." be better to parse?

> The livepatch author is responsible for preventing incompatible
> changes.
> 
> Signed-off-by: Petr Mladek <pmladek@suse.com>
> ---
>  include/linux/livepatch.h | 15 +++++++++
>  kernel/livepatch/Makefile |  2 +-
>  kernel/livepatch/state.c  | 85 +++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 101 insertions(+), 1 deletion(-)
>  create mode 100644 kernel/livepatch/state.c
> 
> diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
> index 273400814020..9c8b637f17cd 100644
> --- a/include/linux/livepatch.h
> +++ b/include/linux/livepatch.h
> @@ -130,10 +130,21 @@ struct klp_object {
>  	bool patched;
>  };
>  
> +/**
> + * struct klp_state - state of the system modified by the livepatch
> + * @id:		system state identifier (non zero)
> + * @data:	custom data
> + */
> +struct klp_state {
> +	unsigned long id;
> +	void *data;
> +};
> +
>  /**
>   * struct klp_patch - patch structure for live patching
>   * @mod:	reference to the live patch module
>   * @objs:	object entries for kernel objects to be patched
> + * @states:	system states that can get modified
>   * @replace:	replace all actively used patches
>   * @list:	list node for global list of actively used patches
>   * @kobj:	kobject for sysfs resources
> @@ -147,6 +158,7 @@ struct klp_patch {
>  	/* external */
>  	struct module *mod;
>  	struct klp_object *objs;
> +	struct klp_state *states;
>  	bool replace;
>  
>  	/* internal */
> @@ -217,6 +229,9 @@ void *klp_shadow_get_or_alloc(void *obj, unsigned long id,
>  void klp_shadow_free(void *obj, unsigned long id, klp_shadow_dtor_t dtor);
>  void klp_shadow_free_all(unsigned long id, klp_shadow_dtor_t dtor);
>  
> +struct klp_state *klp_get_state(struct klp_patch *patch, unsigned long id);
> +struct klp_state *klp_get_prev_state(unsigned long id);
> +
>  #else /* !CONFIG_LIVEPATCH */
>  
>  static inline int klp_module_coming(struct module *mod) { return 0; }
> diff --git a/kernel/livepatch/Makefile b/kernel/livepatch/Makefile
> index cf9b5bcdb952..cf03d4bdfc66 100644
> --- a/kernel/livepatch/Makefile
> +++ b/kernel/livepatch/Makefile
> @@ -1,4 +1,4 @@
>  # SPDX-License-Identifier: GPL-2.0-only
>  obj-$(CONFIG_LIVEPATCH) += livepatch.o
>  
> -livepatch-objs := core.o patch.o shadow.o transition.o
> +livepatch-objs := core.o patch.o shadow.o state.o transition.o
> diff --git a/kernel/livepatch/state.c b/kernel/livepatch/state.c
> new file mode 100644
> index 000000000000..f76d90e856b1
> --- /dev/null
> +++ b/kernel/livepatch/state.c
> @@ -0,0 +1,85 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * system_state.c - State of the system modified by livepatches
> + *
> + * Copyright (C) 2019 SUSE
> + */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/livepatch.h>
> +#include "core.h"
> +#include "transition.h"
> +
> +#define klp_for_each_state(patch, state)		\
> +	for (state = patch->states;			\
> +	     state && state->id;			\
> +	     state++)
> +
> +/**
> + * klp_get_state() - get information about system state modified by
> + *	the given patch
> + * @patch:	livepatch that modifies the given system state
> + * @id:		custom identifier of the modified system state
> + *
> + * Checks whether the given patch modifies to given system state.

s/to given/the given/ ?

Miroslav

  reply index

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-19  7:40 [PATCH v2 0/5] livepatch: new " Petr Mladek
2019-07-19  7:40 ` [PATCH v2 1/5] livepatch: Keep replaced patches until post_patch callback is called Petr Mladek
2019-08-13 13:24   ` Miroslav Benes
2019-07-19  7:40 ` [PATCH v2 2/5] livepatch: Basic API to track system state changes Petr Mladek
2019-08-13 13:33   ` Miroslav Benes [this message]
2019-07-19  7:40 ` [PATCH v2 3/5] livepatch: Allow to distinguish different version of " Petr Mladek
2019-08-13 13:36   ` Miroslav Benes
2019-07-19  7:40 ` [PATCH v2 4/5] livepatch: Documentation of the new API for tracking " Petr Mladek
2019-08-13 13:43   ` Miroslav Benes
2019-07-19  7:40 ` [PATCH v2 5/5] livepatch: Selftests of the " Petr Mladek
2019-08-13 13:46 ` [PATCH v2 0/5] livepatch: new API to track " Miroslav Benes

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.LSU.2.21.1908131525480.10477@pobox.suse.cz \
    --to=mbenes@suse.cz \
    --cc=jikos@kernel.org \
    --cc=joe.lawrence@redhat.com \
    --cc=jpoimboe@redhat.com \
    --cc=kamalesh@linux.vnet.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=live-patching@vger.kernel.org \
    --cc=nstange@suse.de \
    --cc=pmladek@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Live-Patching Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/live-patching/0 live-patching/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 live-patching live-patching/ https://lore.kernel.org/live-patching \
		live-patching@vger.kernel.org
	public-inbox-index live-patching

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.live-patching


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git