Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1259930 > unrolled thread

[PATCH] livepatch: old_name@old_addr scheme in livepatch sysfs directory

Back to article view | Back to linux.kernel

Contents

  [PATCH] livepatch: old_name@old_addr scheme in livepatch sysfs directory Chris J Arges <chris.j.arges@canonical.com> - 2015-10-31 04:50 +0100
    Re: livepatch: old_name@old_addr scheme in livepatch sysfs directory Jessica Yu <jeyu@redhat.com> - 2015-11-01 03:00 +0100
      Re: livepatch: old_name@old_addr scheme in livepatch sysfs directory Chris J Arges <chris.j.arges@canonical.com> - 2015-11-01 05:10 +0100
        Re: livepatch: old_name@old_addr scheme in livepatch sysfs  directory Jiri Kosina <jikos@kernel.org> - 2015-11-01 10:10 +0100
          Re: livepatch: old_name@old_addr scheme in livepatch sysfs directory Jessica Yu <jeyu@redhat.com> - 2015-11-02 10:10 +0100

#1259930 — [PATCH] livepatch: old_name@old_addr scheme in livepatch sysfs directory

The following directory structure will allow for cases when the same
function name exists in a single object.
	/sys/kernel/livepatch/<patch>/<object>/<function@address>

An example of this issue is documented here:
	https://github.com/dynup/kpatch/issues/493

Signed-off-by: Chris J Arges <chris.j.arges@canonical.com>
---
 Documentation/ABI/testing/sysfs-kernel-livepatch | 4 ++--
 kernel/livepatch/core.c                          | 3 ++-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-kernel-livepatch b/Documentation/ABI/testing/sysfs-kernel-livepatch
index 5bf42a8..afa0470 100644
--- a/Documentation/ABI/testing/sysfs-kernel-livepatch
+++ b/Documentation/ABI/testing/sysfs-kernel-livepatch
@@ -33,12 +33,12 @@ Description:
 		The object directory contains subdirectories for each function
 		that is patched within the object.
 
-What:		/sys/kernel/livepatch/<patch>/<object>/<function>
+What:		/sys/kernel/livepatch/<patch>/<object>/<function@address>
 Date:		Nov 2014
 KernelVersion:	3.19.0
 Contact:	live-patching@vger.kernel.org
 Description:
-		The function directory contains attributes regarding the
+		The function@address directory contains attributes regarding the
 		properties and state of the patched function.
 
 		There are currently no such attributes.
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index 6e53441..347cb3e 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -733,7 +733,8 @@ static int klp_init_func(struct klp_object *obj, struct klp_func *func)
 	func->state = KLP_DISABLED;
 
 	return kobject_init_and_add(&func->kobj, &klp_ktype_func,
-				    &obj->kobj, "%s", func->old_name);
+				    &obj->kobj, "%s@%p", func->old_name,
+				    (void *)func->old_addr);
 }
 
 /* parts of the initialization that is done only when the object is loaded */
-- 
1.9.1

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

[toc] | [next] | [standalone]

#1260076 — Re: livepatch: old_name@old_addr scheme in livepatch sysfs directory

+++ Chris J Arges [30/10/15 22:44 -0500]:
>The following directory structure will allow for cases when the same
>function name exists in a single object.
>	/sys/kernel/livepatch/<patch>/<object>/<function@address>

Hi Chris, thanks for the patch.

I think the last time this issue was discussed, the conclusion was
that concatenating the address to the function name constitutes as an
information leak (as the sysfs entry is visible to non-root users).

One option suggested by Josh in that thread would be to do something
like "func.n", where n is just the nth occurrence of the symbol name. 

Another option might be to keep the func@addr format but not make these
entries visible to non-root users.

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

[toc] | [prev] | [next] | [standalone]

#1260077 — Re: livepatch: old_name@old_addr scheme in livepatch sysfs directory

On 10/31/2015 08:53 PM, Jessica Yu wrote:
> +++ Chris J Arges [30/10/15 22:44 -0500]:
>> The following directory structure will allow for cases when the same
>> function name exists in a single object.
>>     /sys/kernel/livepatch/<patch>/<object>/<function@address>
> 
> Hi Chris, thanks for the patch.
> 
> I think the last time this issue was discussed, the conclusion was
> that concatenating the address to the function name constitutes as an
> information leak (as the sysfs entry is visible to non-root users).
> 
> One option suggested by Josh in that thread would be to do something
> like "func.n", where n is just the nth occurrence of the symbol name.
> Another option might be to keep the func@addr format but not make these
> entries visible to non-root users.
> 
> Jessica
> 

Jessica,

Makes sense to me. Is there a reason why the sysfs entries are visible
to non-root users?

Otherwise, if there is a use-case for keeping the permissions the same,
then I'd be happy to to use the 'func.n' format for v2.

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

[toc] | [prev] | [next] | [standalone]

#1260103 — Re: livepatch: old_name@old_addr scheme in livepatch sysfs directory

On Sat, 31 Oct 2015, Chris J Arges wrote:

> Makes sense to me. Is there a reason why the sysfs entries are visible
> to non-root users?

Well, kptr_restrict applies only to values printed using '%pK'. So if the 
sysfs handler is using other printk() format string to print the pointer, 
it'll be leaked out (and should eventually be fixed).

Thanks,

-- 
Jiri Kosina
SUSE Labs

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

[toc] | [prev] | [next] | [standalone]

#1260465 — Re: livepatch: old_name@old_addr scheme in livepatch sysfs directory

+++ Jiri Kosina [01/11/15 10:07 +0100]:
>On Sat, 31 Oct 2015, Chris J Arges wrote:
>
>> Makes sense to me. Is there a reason why the sysfs entries are visible
>> to non-root users?
>
>Well, kptr_restrict applies only to values printed using '%pK'. So if the
>sysfs handler is using other printk() format string to print the pointer,
>it'll be leaked out (and should eventually be fixed).

Hm, I think the func.n solution might be a good temporary fix for now.
Even if we do go with func@addr and use the '%pK' format specifier we
will run into the same problem at kptr_restrict == 2.

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web