[PATCH] tools/livepatch: recognize functions in livepatch.ignore.functions

[PATCH] tools/livepatch: recognize functions in livepatch.ignore.functions

[elena.ufimtseva]

From: Elena Ufimtseva <elena.ufimtseva@oracle.com>

It is expected that the symbol has type STT_FUNC in livpatch.ignore.functions
sections, but it is incorrect and results in functions not to be ignored.
To actually ignore functions in livepatch.ignore.functions section, attempt to
find the symbol of type STT_FUNC by its name.

Signed-off-by: Elena Ufimtseva <elena.ufimtseva@oracle.com>
---
 create-diff-object.c | 27 +++++++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/create-diff-object.c b/create-diff-object.c
index 82f777e..7e845da 100644
--- a/create-diff-object.c
+++ b/create-diff-object.c
@@ -859,6 +859,7 @@ static void kpatch_mark_ignored_functions_same(struct kpatch_elf *kelf)
 {
 	struct section *sec;
 	struct rela *rela;
+	struct symbol *strsym;
 
 	sec = find_section_by_name(&kelf->sections, ".livepatch.ignore.functions");
 	if (!sec)
@@ -867,8 +868,30 @@ static void kpatch_mark_ignored_functions_same(struct kpatch_elf *kelf)
 	list_for_each_entry(rela, &sec->rela->relas, list) {
 		if (!rela->sym->sec)
 			ERROR("expected bundled symbol");
-		if (rela->sym->type != STT_FUNC)
-			ERROR("expected function symbol");
+		if (rela->sym->type != STT_FUNC) {
+			log_debug("expected function symbol and we have %d, name %s", rela->sym->type, rela->sym->name);
+			if (rela->string) {
+				log_debug(", rela string %s\n", rela->string);
+				strsym = find_symbol_by_name(&kelf->symbols, rela->string);
+				if (!strsym)
+					ERROR("can't find %s symbol to ignore\n", rela->string);
+				else {
+					if (strsym->type != STT_FUNC)
+						ERROR("symbol %s is not function to ignore\n", strsym->name);
+					else {
+						strsym->status = SAME;
+						strsym->sec->status = SAME;
+						if (strsym->sec->secsym)
+							strsym->sec->secsym->status = SAME;
+						if (strsym->sec->rela)
+							strsym->sec->rela->status = SAME;
+					}
+				}
+			}
+			log_debug("\n");
+			continue;
+                }
+
 		log_normal("ignoring function: %s\n", rela->sym->name);
 		if (rela->sym->status != CHANGED)
 			log_normal("NOTICE: no change detected in function %s, unnecessary KPATCH_IGNORE_FUNCTION()?\n", rela->sym->name);
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] tools/livepatch: recognize functions in livepatch.ignore.functions

[Ross Lagerwall]

On 12/12/2017 05:54 AM, elena.ufimtseva@oracle.com wrote:
> From: Elena Ufimtseva <elena.ufimtseva@oracle.com>
>
> It is expected that the symbol has type STT_FUNC in livpatch.ignore.functions
> sections, but it is incorrect and results in functions not to be ignored.
> To actually ignore functions in livepatch.ignore.functions section, attempt to
> find the symbol of type STT_FUNC by its name.
>

Hi Elena,

I suspect you might have got the wrong idea about how it is meant to be 
used (or I misunderstood your patch). It is expected that the relocation 
points to a symbol that has type STT_FUNC. This is subtly different from 
having a symbol with type STT_FUNC in .livepatch.ignore.functions.

The correct way to use it is to declare a pointer stored in 
.livepatch.ignore.functions that points to the function you want to ignore.

For example, place this at the end of arch/x86/mm/p2m.c to ignore 
changes to map_domain_gfn:
void *__lp_ignore_func_map_domain_gfn 
__section(.livepatch.ignore.functions) = map_domain_gfn;

The code in livepatch-build-tools is based on kpatch and they have a 
macro to do this in a more friendly way [1]. If you want, it would be 
great if you could port this to Xen LivePatch.

[1] 
https://github.com/dynup/kpatch/blob/f4c0f3209e8e856d93622344560c8794cd8d8a45/kmod/patch/kpatch-macros.h#L39

As an aside, much of the code in livepatch-build-tools comes from 
kpatch, so patches are ideally sent there first. We do intend eventually 
to merge the tools to avoid code duplication and kpatch's build tool was 
recently refactored to make this easier.

Cheers,
-- 
Ross Lagerwall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] tools/livepatch: recognize functions in livepatch.ignore.functions

[Elena Ufimtseva]

Hi Ross

Thanks for the info.
I actually did look into the KPATCH_IGNORE_FUNCTION code. But.. I
somehow ended up having this:

#define KPATCH_IGNORE_FUNCTION(_fn) \
void *__kpatch_ignore_func_##_fn __section(.kpatch.ignore.functions) = (#_fn);

Which apparently caused the symbol not being a function type, but a string(?).

Ok, thanks, I will be now dealing with this "artifact", ugh.

Elena


On Tue, Dec 12, 2017 at 12:09 AM, Ross Lagerwall
<ross.lagerwall@citrix.com> wrote:
> On 12/12/2017 05:54 AM, elena.ufimtseva@oracle.com wrote:
>>
>> From: Elena Ufimtseva <elena.ufimtseva@oracle.com>
>>
>> It is expected that the symbol has type STT_FUNC in
>> livpatch.ignore.functions
>> sections, but it is incorrect and results in functions not to be ignored.
>> To actually ignore functions in livepatch.ignore.functions section,
>> attempt to
>> find the symbol of type STT_FUNC by its name.
>>
>
> Hi Elena,
>
> I suspect you might have got the wrong idea about how it is meant to be used
> (or I misunderstood your patch). It is expected that the relocation points
> to a symbol that has type STT_FUNC. This is subtly different from having a
> symbol with type STT_FUNC in .livepatch.ignore.functions.
>
> The correct way to use it is to declare a pointer stored in
> .livepatch.ignore.functions that points to the function you want to ignore.
>
> For example, place this at the end of arch/x86/mm/p2m.c to ignore changes to
> map_domain_gfn:
> void *__lp_ignore_func_map_domain_gfn __section(.livepatch.ignore.functions)
> = map_domain_gfn;
>
> The code in livepatch-build-tools is based on kpatch and they have a macro
> to do this in a more friendly way [1]. If you want, it would be great if you
> could port this to Xen LivePatch.
>
> [1]
> https://github.com/dynup/kpatch/blob/f4c0f3209e8e856d93622344560c8794cd8d8a45/kmod/patch/kpatch-macros.h#L39
>
> As an aside, much of the code in livepatch-build-tools comes from kpatch, so
> patches are ideally sent there first. We do intend eventually to merge the
> tools to avoid code duplication and kpatch's build tool was recently
> refactored to make this easier.
>
> Cheers,
> --
> Ross Lagerwall
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel



-- 
Elena

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] tools/livepatch: recognize functions in livepatch.ignore.functions

[Ross Lagerwall]

On 12/14/2017 07:18 AM, Elena Ufimtseva wrote:
> Hi Ross
>
> Thanks for the info.
> I actually did look into the KPATCH_IGNORE_FUNCTION code. But.. I
> somehow ended up having this:
>
> #define KPATCH_IGNORE_FUNCTION(_fn) \
> void *__kpatch_ignore_func_##_fn __section(.kpatch.ignore.functions) = (#_fn);
>
> Which apparently caused the symbol not being a function type, but a string(?).
>

You've got a single # before _fn which causes stringification of the 
macro parameter. Therefore the symbol became a string and not a function.

Cheers,
-- 
Ross Lagerwall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel