[PATCH] livepatch: add sysfs interface /sys/kernel/livepatch/state

[PATCH] livepatch: add sysfs interface /sys/kernel/livepatch/state

[Li Bin]

The added sysfs interface /sys/kernel/livepatch/state is read-only,
it shows the patches that have been applied, incluing the stack index
and the state of each patch.

$ cat /sys/kernel/livepatch/state
Index	Patch				State
-----------------------------------------------
1	klp_test1			enabled
2	klp_test2			enabled
3	klp_test3			enabled
-----------------------------------------------

$ echo 0 > /sys/kernel/livepatch/klp_test3/enabled
$ cat /sys/kernel/livepatch/state
Index	Patch				State
-----------------------------------------------
1	klp_test1			enabled
2	klp_test2			enabled
3	klp_test3			disabled
-----------------------------------------------

Signed-off-by: Li Bin <huawei.libin@huawei.com>
---
 Documentation/ABI/testing/sysfs-kernel-livepatch |    9 +++++
 kernel/livepatch/core.c                          |   41 +++++++++++++++++++++-
 2 files changed, 49 insertions(+), 1 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-kernel-livepatch b/Documentation/ABI/testing/sysfs-kernel-livepatch
index 5bf42a8..01c64da 100644
--- a/Documentation/ABI/testing/sysfs-kernel-livepatch
+++ b/Documentation/ABI/testing/sysfs-kernel-livepatch
@@ -8,6 +8,15 @@ Description:
 		The /sys/kernel/livepatch directory contains subdirectories for
 		each loaded live patch module.
 
+What:		/sys/kernel/livepatch/state
+Date:		Jun 2015
+KernelVersion:	4.1.0	
+Contact:	live-patching@vger.kernel.org
+Description:
+		The state file is read-only and shows the patches that have
+		been applied, including the patch stack index and state of
+		each patch. 
+
 What:		/sys/kernel/livepatch/<patch>
 Date:		Nov 2014
 KernelVersion:	3.19.0
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index 3f9f1d6..5d004f3 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -604,6 +604,7 @@ EXPORT_SYMBOL_GPL(klp_enable_patch);
  * Sysfs Interface
  *
  * /sys/kernel/livepatch
+ * /sys/kernel/livepatch/state
  * /sys/kernel/livepatch/<patch>
  * /sys/kernel/livepatch/<patch>/enabled
  * /sys/kernel/livepatch/<patch>/<object>
@@ -1008,6 +1009,36 @@ static struct notifier_block klp_module_nb = {
 	.priority = INT_MIN+1, /* called late but before ftrace notifier */
 };
 
+static ssize_t state_show(struct kobject *kobj,
+			    struct kobj_attribute *attr, char *buf)
+{
+	struct klp_patch *patch;
+	unsigned long size = 0;
+	int index = 0;
+	size += snprintf(buf+size, PAGE_SIZE-size-1, "%-5s\t%-26s\t%-8s\n", "Index", "Patch", "State");
+	size += snprintf(buf+size, PAGE_SIZE-size-1, "-----------------------------------------------\n");
+	mutex_lock(&klp_mutex);
+	list_for_each_entry(patch, &klp_patches, list) {
+		size += snprintf(buf+size, PAGE_SIZE-size-1, "%-5d\t%-26s\t%-8s\n",
+			++index, patch->mod->name, patch->state ? "enabled" : "disabled");
+	}
+	mutex_unlock(&klp_mutex);
+	size += snprintf(buf+size, PAGE_SIZE-size-1, "-----------------------------------------------\n");
+
+	return size;
+}
+static struct kobj_attribute state_kobj_attr = __ATTR_RO(state);
+
+static struct attribute *klp_top_attrs[] = {
+	&state_kobj_attr.attr,
+	NULL
+};
+
+static struct kobj_type klp_ktype_top = {
+	.sysfs_ops = &kobj_sysfs_ops,
+	.default_attrs = klp_top_attrs,
+};
+
 static int klp_init(void)
 {
 	int ret;
@@ -1022,12 +1053,20 @@ static int klp_init(void)
 	if (ret)
 		return ret;
 
-	klp_root_kobj = kobject_create_and_add("livepatch", kernel_kobj);
+	klp_root_kobj = kzalloc(sizeof(*klp_root_kobj), GFP_KERNEL);
 	if (!klp_root_kobj) {
 		ret = -ENOMEM;
 		goto unregister;
 	}
 
+	ret = kobject_init_and_add(klp_root_kobj, &klp_ktype_top,
+			kernel_kobj, "livepatch");
+	if (ret) {
+		kobject_put(klp_root_kobj);
+		klp_root_kobj = NULL;
+		goto unregister;
+	}
+
 	return 0;
 
 unregister:
-- 
1.7.7

Re: [PATCH] livepatch: add sysfs interface /sys/kernel/livepatch/state

[Miroslav Benes]

On Wed, 17 Jun 2015, Li Bin wrote:

> The added sysfs interface /sys/kernel/livepatch/state is read-only,
> it shows the patches that have been applied, incluing the stack index
> and the state of each patch.
> 
> $ cat /sys/kernel/livepatch/state
> Index	Patch				State
> -----------------------------------------------
> 1	klp_test1			enabled
> 2	klp_test2			enabled
> 3	klp_test3			enabled
> -----------------------------------------------
> 
> $ echo 0 > /sys/kernel/livepatch/klp_test3/enabled
> $ cat /sys/kernel/livepatch/state
> Index	Patch				State
> -----------------------------------------------
> 1	klp_test1			enabled
> 2	klp_test2			enabled
> 3	klp_test3			disabled
> -----------------------------------------------
> 
> Signed-off-by: Li Bin <huawei.libin@huawei.com>
> ---

Hi,

I think we should comply with sysfs policy and keep 'one value per 
attribute' as mentioned in the Documentation/filesystems/sysfs.txt:

"
Attributes should be ASCII text files, preferably with only one value
per file. It is noted that it may not be efficient to contain only one
value per file, so it is socially acceptable to express an array of
values of the same type.

Mixing types, expressing multiple lines of data, and doing fancy
formatting of data is heavily frowned upon. Doing these things may get
you publicly humiliated and your code rewritten without notice.
"
The list of applied patches can be obtained just by 'ls 
/sys/kernel/livepatch' and their state is in enabled attribute in each 
respective patch (no, you cannot obtain the order in the stack).

Thanks,

Miroslav Benes
SUSE Labs

Re: [PATCH] livepatch: add sysfs interface /sys/kernel/livepatch/state

[Li Bin]

On 2015/6/17 16:13, Miroslav Benes wrote:
> On Wed, 17 Jun 2015, Li Bin wrote:
> 
>> The added sysfs interface /sys/kernel/livepatch/state is read-only,
>> it shows the patches that have been applied, incluing the stack index
>> and the state of each patch.
>>
>> $ cat /sys/kernel/livepatch/state
>> Index	Patch				State
>> -----------------------------------------------
>> 1	klp_test1			enabled
>> 2	klp_test2			enabled
>> 3	klp_test3			enabled
>> -----------------------------------------------
>>
>> $ echo 0 > /sys/kernel/livepatch/klp_test3/enabled
>> $ cat /sys/kernel/livepatch/state
>> Index	Patch				State
>> -----------------------------------------------
>> 1	klp_test1			enabled
>> 2	klp_test2			enabled
>> 3	klp_test3			disabled
>> -----------------------------------------------
>>
>> Signed-off-by: Li Bin <huawei.libin@huawei.com>
>> ---
> 
> Hi,
> 
> I think we should comply with sysfs policy and keep 'one value per 
> attribute' as mentioned in the Documentation/filesystems/sysfs.txt:
> 
> "
> Attributes should be ASCII text files, preferably with only one value
> per file. It is noted that it may not be efficient to contain only one
> value per file, so it is socially acceptable to express an array of
> values of the same type.
> 
> Mixing types, expressing multiple lines of data, and doing fancy
> formatting of data is heavily frowned upon. Doing these things may get
> you publicly humiliated and your code rewritten without notice.
> "

Hi Miroslav Benes,
Thanks for your comments.

> The list of applied patches can be obtained just by 'ls 
> /sys/kernel/livepatch' and their state is in enabled attribute in each 
> respective patch (no, you cannot obtain the order in the stack).

But why we cannot obtain it? I think We indeed need the stack order when we
will disable one patch, at least, we can find out whether it is on the top of
the stack if failed to disable one patch.

Thanks,
	Li Bin
> 
> Thanks,
> 
> Miroslav Benes
> SUSE Labs
> 
> .
> 

Re: [PATCH] livepatch: add sysfs interface /sys/kernel/livepatch/state

[Miroslav Benes]

On Wed, 17 Jun 2015, Li Bin wrote:

> On 2015/6/17 16:13, Miroslav Benes wrote:
> > On Wed, 17 Jun 2015, Li Bin wrote:
>
> > The list of applied patches can be obtained just by 'ls 
> > /sys/kernel/livepatch' and their state is in enabled attribute in each 
> > respective patch (no, you cannot obtain the order in the stack).
> 
> But why we cannot obtain it? I think We indeed need the stack order when we
> will disable one patch, at least, we can find out whether it is on the top of
> the stack if failed to disable one patch.

I meant with the current means. It is correct that we do not export 
information about stacking order anywhere.

What we do in kGraft is that there is something like refcount for each 
patch. When the patch is being applied the refcount of all the previous 
patches is increased. Only the patch with the refcount equal to 0 can be 
removed. This information is exported and gives one a clue about the 
order.

So if there is a need to have something like this there would certainly 
be a way (or ways to be precise) how to do it. The question is if we need 
it right now.

Regards,
Miroslav

Re: [PATCH] livepatch: add sysfs interface /sys/kernel/livepatch/state

[Li Bin]

On 2015/6/17 21:20, Miroslav Benes wrote:
> On Wed, 17 Jun 2015, Li Bin wrote:
> 
>> On 2015/6/17 16:13, Miroslav Benes wrote:
>>> On Wed, 17 Jun 2015, Li Bin wrote:
>>
>>> The list of applied patches can be obtained just by 'ls 
>>> /sys/kernel/livepatch' and their state is in enabled attribute in each 
>>> respective patch (no, you cannot obtain the order in the stack).
>>
>> But why we cannot obtain it? I think We indeed need the stack order when we
>> will disable one patch, at least, we can find out whether it is on the top of
>> the stack if failed to disable one patch.
> 
> I meant with the current means. It is correct that we do not export 
> information about stacking order anywhere.
> 
> What we do in kGraft is that there is something like refcount for each 
> patch. When the patch is being applied the refcount of all the previous 
> patches is increased. Only the patch with the refcount equal to 0 can be 
> removed. This information is exported and gives one a clue about the 
> order.
> 

It sounds good, but the information is limited that cannot show the stack
order, right? (The refcount of all the disabled patch is equal to 0, if
being enable one disabled patch, the stack order is also needed.)

refcount Patch
-------------------
3	patch1(enabled)
2	patch2(enabled)
1	patch3(enabled)
0	patch4(enabled)
0	patch5(disabled)
0	patch6(disabled)

Unless the refcount is allowed to be less than 0, then when the patch is
being disabled the refcount of all the patches is decreased, when the patch
is being enabled the refcount of all patches is increased. Only the patch
with the refcount equal to 0 can be disabled and only equal to -1 can be
enabled, and only less or equal to 0 can be removed (that the livepatch does
not support right now).

refcount Patch
-------------------
3	patch1(enabled)
2	patch2(enabled)
1	patch3(enabled)
0	patch4(enabled)
-1	patch5(disabled)
-2	patch6(disabled)

Thanks,
	Li Bin

> So if there is a need to have something like this there would certainly 
> be a way (or ways to be precise) how to do it. The question is if we need 
> it right now.
> 
> Regards,
> Miroslav
> 
> .
>