diff mbox series

[livepatch-build-tools,part2,5/6] create-diff-object: Add new entries to special sections array

Message ID 20190416120716.26269-5-wipawel@amazon.de (mailing list archive)
State New, archived
Headers show
Series [livepatch-build-tools,part2,1/6] common: Add is_standard_section() helper function | expand

Commit Message

Wieczorkiewicz, Pawel April 16, 2019, 12:07 p.m. UTC
Handle .livepatch.hooks* and .altinstr_replacement sections as the
special sections with assigned group_size resolution function.
By default each .livepatch.hooks* sections' entry is 8 bytes long (a
pointer). The .altinstr_replacement section follows the .altinstructions
section settings.

Allow to specify different .livepatch.hooks* section entry size using
shell environment variable HOOK_STRUCT_SIZE.

Cleanup an incorrect indentation around group_size resulution functions.

Signed-off-by: Pawel Wieczorkiewicz <wipawel@amazon.de>
Reviewed-by: Andra-Irina Paraschiv <andraprs@amazon.com>
Reviewed-by: Bjoern Doebel <doebel@amazon.de>
Reviewed-by: Norbert Manthey <nmanthey@amazon.de>
---
 create-diff-object.c | 87 ++++++++++++++++++++++++++++++++--------------------
 1 file changed, 54 insertions(+), 33 deletions(-)

Comments

Ross Lagerwall April 29, 2019, 3:47 p.m. UTC | #1
On 4/16/19 1:07 PM, Pawel Wieczorkiewicz wrote:
> Handle .livepatch.hooks* and .altinstr_replacement sections as the
> special sections with assigned group_size resolution function.
> By default each .livepatch.hooks* sections' entry is 8 bytes long (a
> pointer). The .altinstr_replacement section follows the .altinstructions
> section settings.
> 
> Allow to specify different .livepatch.hooks* section entry size using
> shell environment variable HOOK_STRUCT_SIZE.
> 
> Cleanup an incorrect indentation around group_size resulution functions.
> 
> Signed-off-by: Pawel Wieczorkiewicz <wipawel@amazon.de>
> Reviewed-by: Andra-Irina Paraschiv <andraprs@amazon.com>
> Reviewed-by: Bjoern Doebel <doebel@amazon.de>
> Reviewed-by: Norbert Manthey <nmanthey@amazon.de>
> ---
>   create-diff-object.c | 87 ++++++++++++++++++++++++++++++++--------------------
>   1 file changed, 54 insertions(+), 33 deletions(-)
> 
> diff --git a/create-diff-object.c b/create-diff-object.c
> index b0b4dcb..f6060cd 100644
> --- a/create-diff-object.c
> +++ b/create-diff-object.c
> @@ -960,51 +960,64 @@ static void kpatch_mark_constant_labels_same(struct kpatch_elf *kelf)

Fix the indentation in the patch which introduces the problem rather 
than fixing it in a subsequent patch.

>   
>   static int bug_frames_group_size(struct kpatch_elf *kelf, int offset)
>   {
> -    static int size = 0;
> -    char *str;
> -    if (!size) {
> -        str = getenv("BUG_STRUCT_SIZE");
> -        size = str ? atoi(str) : 8;
> -    }
> -
> -    return size;
> +	static int size = 0;
> +	char *str;
> +	if (!size) {
> +		str = getenv("BUG_STRUCT_SIZE");
> +		size = str ? atoi(str) : 8;
> +	}
> +
> +	return size;
>   }
>   
>   static int bug_frames_3_group_size(struct kpatch_elf *kelf, int offset)
>   {
> -    static int size = 0;
> -    char *str;
> -    if (!size) {
> -        str = getenv("BUG_STRUCT_SIZE");
> -        size = str ? atoi(str) : 16;
> -    }
> -
> -    return size;
> +	static int size = 0;
> +	char *str;
> +	if (!size) {
> +		str = getenv("BUG_STRUCT_SIZE");
> +		size = str ? atoi(str) : 16;
> +	}
> +
> +	return size;
>   }
>   
>   static int ex_table_group_size(struct kpatch_elf *kelf, int offset)
>   {
> -    static int size = 0;
> -    char *str;
> -    if (!size) {
> -        str = getenv("EX_STRUCT_SIZE");
> -        size = str ? atoi(str) : 8;
> -    }
> -
> -    return size;
> +	static int size = 0;
> +	char *str;
> +	if (!size) {
> +		str = getenv("EX_STRUCT_SIZE");
> +		size = str ? atoi(str) : 8;
> +	}
> +
> +	return size;
>   }
>   
>   static int altinstructions_group_size(struct kpatch_elf *kelf, int offset)
>   {
> -    static int size = 0;
> -    char *str;
> -    if (!size) {
> -        str = getenv("ALT_STRUCT_SIZE");
> -        size = str ? atoi(str) : 12;
> -    }
> -
> -    printf("altinstr_size=%d\n", size);
> -    return size;
> +	static int size = 0;
> +	char *str;
> +	if (!size) {
> +		str = getenv("ALT_STRUCT_SIZE");
> +		size = str ? atoi(str) : 12;
> +	}
> +
> +	printf("altinstr_size=%d\n", size);
> +	return size;
> +}
> +
> +static int livepatch_hooks_group_size(struct kpatch_elf *kelf, int offset)
> +{
> +	static int size = 0;
> +	char *str;
> +	if (!size) {
> +		str = getenv("HOOK_STRUCT_SIZE");
> +		size = str ? atoi(str) : 8;
> +	}
> +
> +	printf("livepatch_hooks_size=%d\n", size);

If you want to keep this debugging output, rather use log_debug().

> +	return size;
>   }
>   
>   /*
> @@ -1084,6 +1097,14 @@ static struct special_section special_sections[] = {
>   		.name		= ".altinstructions",
>   		.group_size	= altinstructions_group_size,
>   	},
> +	{
> +		.name		= ".altinstr_replacement",
> +		.group_size	= altinstructions_group_size,
> +	},
> +	{
> +		.name		= ".livepatch.hooks",
> +		.group_size	= livepatch_hooks_group_size,
> +	},
>   	{},
>   };
>   
> 

I don't understand the point of this change.

IIUC unlike .altinstructions, the .altinstr_replacement section does not 
have a fixed group size, so this change would not work other than by 
luck. If the goal was to prune bits of the .altinstr_replacement table 
that do not need to be included, you need to update the code in 
kpatch_process_special_sections() which has special handling for 
.altinstr_replacement. It needs to parse the updated, pruned 
.altinstructions section and regenerate .altinstr_replacement, including 
only what is needed.

I don't know why .livepatch.hooks is included in this list either since 
all hooks are always included and there is nothing to regenerate/prune.

Regards,
Wieczorkiewicz, Pawel April 30, 2019, 1:01 p.m. UTC | #2
On 29. Apr 2019, at 17:47, Ross Lagerwall <ross.lagerwall@citrix.com<mailto:ross.lagerwall@citrix.com>> wrote:

On 4/16/19 1:07 PM, Pawel Wieczorkiewicz wrote:
Handle .livepatch.hooks* and .altinstr_replacement sections as the
special sections with assigned group_size resolution function.
By default each .livepatch.hooks* sections' entry is 8 bytes long (a
pointer). The .altinstr_replacement section follows the .altinstructions
section settings.
Allow to specify different .livepatch.hooks* section entry size using
shell environment variable HOOK_STRUCT_SIZE.
Cleanup an incorrect indentation around group_size resulution functions.
Signed-off-by: Pawel Wieczorkiewicz <wipawel@amazon.de<mailto:wipawel@amazon.de>>
Reviewed-by: Andra-Irina Paraschiv <andraprs@amazon.com<mailto:andraprs@amazon.com>>
Reviewed-by: Bjoern Doebel <doebel@amazon.de<mailto:doebel@amazon.de>>
Reviewed-by: Norbert Manthey <nmanthey@amazon.de<mailto:nmanthey@amazon.de>>
---
 create-diff-object.c | 87 ++++++++++++++++++++++++++++++++--------------------
 1 file changed, 54 insertions(+), 33 deletions(-)
diff --git a/create-diff-object.c b/create-diff-object.c
index b0b4dcb..f6060cd 100644
--- a/create-diff-object.c
+++ b/create-diff-object.c
@@ -960,51 +960,64 @@ static void kpatch_mark_constant_labels_same(struct kpatch_elf *kelf)

Fix the indentation in the patch which introduces the problem rather than fixing it in a subsequent patch.

Oh, certainly. I forgot to update the commits involved properly.

ACK. Will fix.


   static int bug_frames_group_size(struct kpatch_elf *kelf, int offset)
 {
-    static int size = 0;
-    char *str;
-    if (!size) {
-        str = getenv("BUG_STRUCT_SIZE");
-        size = str ? atoi(str) : 8;
-    }
-
-    return size;
+ static int size = 0;
+ char *str;
+ if (!size) {
+ str = getenv("BUG_STRUCT_SIZE");
+ size = str ? atoi(str) : 8;
+ }
+
+ return size;
 }
   static int bug_frames_3_group_size(struct kpatch_elf *kelf, int offset)
 {
-    static int size = 0;
-    char *str;
-    if (!size) {
-        str = getenv("BUG_STRUCT_SIZE");
-        size = str ? atoi(str) : 16;
-    }
-
-    return size;
+ static int size = 0;
+ char *str;
+ if (!size) {
+ str = getenv("BUG_STRUCT_SIZE");
+ size = str ? atoi(str) : 16;
+ }
+
+ return size;
 }
   static int ex_table_group_size(struct kpatch_elf *kelf, int offset)
 {
-    static int size = 0;
-    char *str;
-    if (!size) {
-        str = getenv("EX_STRUCT_SIZE");
-        size = str ? atoi(str) : 8;
-    }
-
-    return size;
+ static int size = 0;
+ char *str;
+ if (!size) {
+ str = getenv("EX_STRUCT_SIZE");
+ size = str ? atoi(str) : 8;
+ }
+
+ return size;
 }
   static int altinstructions_group_size(struct kpatch_elf *kelf, int offset)
 {
-    static int size = 0;
-    char *str;
-    if (!size) {
-        str = getenv("ALT_STRUCT_SIZE");
-        size = str ? atoi(str) : 12;
-    }
-
-    printf("altinstr_size=%d\n", size);
-    return size;
+ static int size = 0;
+ char *str;
+ if (!size) {
+ str = getenv("ALT_STRUCT_SIZE");
+ size = str ? atoi(str) : 12;
+ }
+
+ printf("altinstr_size=%d\n", size);
+ return size;
+}
+
+static int livepatch_hooks_group_size(struct kpatch_elf *kelf, int offset)
+{
+ static int size = 0;
+ char *str;
+ if (!size) {
+ str = getenv("HOOK_STRUCT_SIZE");
+ size = str ? atoi(str) : 8;
+ }
+
+ printf("livepatch_hooks_size=%d\n", size);

If you want to keep this debugging output, rather use log_debug().

ACK. Will fix.


+ return size;
 }
   /*
@@ -1084,6 +1097,14 @@ static struct special_section special_sections[] = {
  .name = ".altinstructions",
  .group_size = altinstructions_group_size,
  },
+ {
+ .name = ".altinstr_replacement",
+ .group_size = altinstructions_group_size,
+ },
+ {
+ .name = ".livepatch.hooks",
+ .group_size = livepatch_hooks_group_size,
+ },
  {},
 };


I don't understand the point of this change.

IIUC unlike .altinstructions, the .altinstr_replacement section does not have a fixed group size, so this change would not work other than by luck. If

That’s true. The .altinstr_replacement group_size should be 0 (aka undefined).

ACK. Will fix.

the goal was to prune bits of the .altinstr_replacement table that do not need to be included, you need to update the code in kpatch_process_special_sections() which has special handling for .altinstr_replacement. It needs to parse the updated, pruned .altinstructions section and regenerate .altinstr_replacement, including only what is needed.


No this was not the goal. I am explaining it below.

I don't know why .livepatch.hooks is included in this list either since all hooks are always included and there is nothing to regenerate/prune.

As discussed when reviewing [1], I need a function telling which section is special.
By special I mean:
a) should not be a subject of extra validation (See [2])
b) should not be a subject of STN_UNDEF purging (See [3])

Thus, I want to explicitly call out all "special" sections by their name and specify
their group_size whenever applicable (I did it incorrectly for .altinstr_replacement,
as you have spotted - thanks!) in the common array and use it with the helper
function or for obtaining a group_size.

[1] [livepatch-build-tools part2 3/6] create-diff-object: Add is_special_section() helper function
[2] [livepatch-build-tools part3 2/3] create-diff-object: Extend patchability verification: STN_UNDEF
[3] [livepatch-build-tools part3 3/3] create-diff-object: Strip all undefined entires of known size


Regards,
--
Ross Lagerwall


Best Regards,
Pawel Wieczorkiewicz



Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrer: Christian Schlaeger, Ralf Herbrich
Ust-ID: DE 289 237 879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class="">
<br class="">
<div>
<blockquote type="cite" class="">
<div class="">On 29. Apr 2019, at 17:47, Ross Lagerwall &lt;<a href="mailto:ross.lagerwall@citrix.com" class="">ross.lagerwall@citrix.com</a>&gt; wrote:</div>
<br class="Apple-interchange-newline">
<div class="">
<div class="">On 4/16/19 1:07 PM, Pawel Wieczorkiewicz wrote:<br class="">
<blockquote type="cite" class="">Handle .livepatch.hooks* and .altinstr_replacement sections as the<br class="">
special sections with assigned group_size resolution function.<br class="">
By default each .livepatch.hooks* sections' entry is 8 bytes long (a<br class="">
pointer). The .altinstr_replacement section follows the .altinstructions<br class="">
section settings.<br class="">
Allow to specify different .livepatch.hooks* section entry size using<br class="">
shell environment variable HOOK_STRUCT_SIZE.<br class="">
Cleanup an incorrect indentation around group_size resulution functions.<br class="">
Signed-off-by: Pawel Wieczorkiewicz &lt;<a href="mailto:wipawel@amazon.de" class="">wipawel@amazon.de</a>&gt;<br class="">
Reviewed-by: Andra-Irina Paraschiv &lt;<a href="mailto:andraprs@amazon.com" class="">andraprs@amazon.com</a>&gt;<br class="">
Reviewed-by: Bjoern Doebel &lt;<a href="mailto:doebel@amazon.de" class="">doebel@amazon.de</a>&gt;<br class="">
Reviewed-by: Norbert Manthey &lt;<a href="mailto:nmanthey@amazon.de" class="">nmanthey@amazon.de</a>&gt;<br class="">
---<br class="">
&nbsp;create-diff-object.c | 87 &#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;&#43;--------------------<br class="">
&nbsp;1 file changed, 54 insertions(&#43;), 33 deletions(-)<br class="">
diff --git a/create-diff-object.c b/create-diff-object.c<br class="">
index b0b4dcb..f6060cd 100644<br class="">
--- a/create-diff-object.c<br class="">
&#43;&#43;&#43; b/create-diff-object.c<br class="">
@@ -960,51 &#43;960,64 @@ static void kpatch_mark_constant_labels_same(struct kpatch_elf *kelf)<br class="">
</blockquote>
<br class="">
Fix the indentation in the patch which introduces the problem rather than fixing it in a subsequent patch.<br class="">
</div>
</div>
</blockquote>
<div><br class="">
</div>
<div>Oh, certainly. I forgot to update the commits involved properly.</div>
<div><br class="">
</div>
<div>ACK. Will fix.</div>
<br class="">
<blockquote type="cite" class="">
<div class="">
<div class=""><br class="">
<blockquote type="cite" class="">&nbsp;&nbsp;&nbsp;static int bug_frames_group_size(struct kpatch_elf *kelf, int offset)<br class="">
&nbsp;{<br class="">
- &nbsp;&nbsp;&nbsp;static int size = 0;<br class="">
- &nbsp;&nbsp;&nbsp;char *str;<br class="">
- &nbsp;&nbsp;&nbsp;if (!size) {<br class="">
- &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;str = getenv(&quot;BUG_STRUCT_SIZE&quot;);<br class="">
- &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;size = str ? atoi(str) : 8;<br class="">
- &nbsp;&nbsp;&nbsp;}<br class="">
-<br class="">
- &nbsp;&nbsp;&nbsp;return size;<br class="">
&#43;<span class="Apple-tab-span" style="white-space:pre"> </span>static int size = 0;<br class="">
&#43;<span class="Apple-tab-span" style="white-space:pre"> </span>char *str;<br class="">
&#43;<span class="Apple-tab-span" style="white-space:pre"> </span>if (!size) {<br class="">
&#43;<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"></span>str = getenv(&quot;BUG_STRUCT_SIZE&quot;);<br class="">
&#43;<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"></span>size = str ? atoi(str) : 8;<br class="">
&#43;<span class="Apple-tab-span" style="white-space:pre"> </span>}<br class="">
&#43;<br class="">
&#43;<span class="Apple-tab-span" style="white-space:pre"> </span>return size;<br class="">
&nbsp;}<br class="">
&nbsp;&nbsp;&nbsp;static int bug_frames_3_group_size(struct kpatch_elf *kelf, int offset)<br class="">
&nbsp;{<br class="">
- &nbsp;&nbsp;&nbsp;static int size = 0;<br class="">
- &nbsp;&nbsp;&nbsp;char *str;<br class="">
- &nbsp;&nbsp;&nbsp;if (!size) {<br class="">
- &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;str = getenv(&quot;BUG_STRUCT_SIZE&quot;);<br class="">
- &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;size = str ? atoi(str) : 16;<br class="">
- &nbsp;&nbsp;&nbsp;}<br class="">
-<br class="">
- &nbsp;&nbsp;&nbsp;return size;<br class="">
&#43;<span class="Apple-tab-span" style="white-space:pre"> </span>static int size = 0;<br class="">
&#43;<span class="Apple-tab-span" style="white-space:pre"> </span>char *str;<br class="">
&#43;<span class="Apple-tab-span" style="white-space:pre"> </span>if (!size) {<br class="">
&#43;<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"></span>str = getenv(&quot;BUG_STRUCT_SIZE&quot;);<br class="">
&#43;<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"></span>size = str ? atoi(str) : 16;<br class="">
&#43;<span class="Apple-tab-span" style="white-space:pre"> </span>}<br class="">
&#43;<br class="">
&#43;<span class="Apple-tab-span" style="white-space:pre"> </span>return size;<br class="">
&nbsp;}<br class="">
&nbsp;&nbsp;&nbsp;static int ex_table_group_size(struct kpatch_elf *kelf, int offset)<br class="">
&nbsp;{<br class="">
- &nbsp;&nbsp;&nbsp;static int size = 0;<br class="">
- &nbsp;&nbsp;&nbsp;char *str;<br class="">
- &nbsp;&nbsp;&nbsp;if (!size) {<br class="">
- &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;str = getenv(&quot;EX_STRUCT_SIZE&quot;);<br class="">
- &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;size = str ? atoi(str) : 8;<br class="">
- &nbsp;&nbsp;&nbsp;}<br class="">
-<br class="">
- &nbsp;&nbsp;&nbsp;return size;<br class="">
&#43;<span class="Apple-tab-span" style="white-space:pre"> </span>static int size = 0;<br class="">
&#43;<span class="Apple-tab-span" style="white-space:pre"> </span>char *str;<br class="">
&#43;<span class="Apple-tab-span" style="white-space:pre"> </span>if (!size) {<br class="">
&#43;<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"></span>str = getenv(&quot;EX_STRUCT_SIZE&quot;);<br class="">
&#43;<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"></span>size = str ? atoi(str) : 8;<br class="">
&#43;<span class="Apple-tab-span" style="white-space:pre"> </span>}<br class="">
&#43;<br class="">
&#43;<span class="Apple-tab-span" style="white-space:pre"> </span>return size;<br class="">
&nbsp;}<br class="">
&nbsp;&nbsp;&nbsp;static int altinstructions_group_size(struct kpatch_elf *kelf, int offset)<br class="">
&nbsp;{<br class="">
- &nbsp;&nbsp;&nbsp;static int size = 0;<br class="">
- &nbsp;&nbsp;&nbsp;char *str;<br class="">
- &nbsp;&nbsp;&nbsp;if (!size) {<br class="">
- &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;str = getenv(&quot;ALT_STRUCT_SIZE&quot;);<br class="">
- &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;size = str ? atoi(str) : 12;<br class="">
- &nbsp;&nbsp;&nbsp;}<br class="">
-<br class="">
- &nbsp;&nbsp;&nbsp;printf(&quot;altinstr_size=%d\n&quot;, size);<br class="">
- &nbsp;&nbsp;&nbsp;return size;<br class="">
&#43;<span class="Apple-tab-span" style="white-space:pre"> </span>static int size = 0;<br class="">
&#43;<span class="Apple-tab-span" style="white-space:pre"> </span>char *str;<br class="">
&#43;<span class="Apple-tab-span" style="white-space:pre"> </span>if (!size) {<br class="">
&#43;<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"></span>str = getenv(&quot;ALT_STRUCT_SIZE&quot;);<br class="">
&#43;<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"></span>size = str ? atoi(str) : 12;<br class="">
&#43;<span class="Apple-tab-span" style="white-space:pre"> </span>}<br class="">
&#43;<br class="">
&#43;<span class="Apple-tab-span" style="white-space:pre"> </span>printf(&quot;altinstr_size=%d\n&quot;, size);<br class="">
&#43;<span class="Apple-tab-span" style="white-space:pre"> </span>return size;<br class="">
&#43;}<br class="">
&#43;<br class="">
&#43;static int livepatch_hooks_group_size(struct kpatch_elf *kelf, int offset)<br class="">
&#43;{<br class="">
&#43;<span class="Apple-tab-span" style="white-space:pre"> </span>static int size = 0;<br class="">
&#43;<span class="Apple-tab-span" style="white-space:pre"> </span>char *str;<br class="">
&#43;<span class="Apple-tab-span" style="white-space:pre"> </span>if (!size) {<br class="">
&#43;<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"></span>str = getenv(&quot;HOOK_STRUCT_SIZE&quot;);<br class="">
&#43;<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"></span>size = str ? atoi(str) : 8;<br class="">
&#43;<span class="Apple-tab-span" style="white-space:pre"> </span>}<br class="">
&#43;<br class="">
&#43;<span class="Apple-tab-span" style="white-space:pre"> </span>printf(&quot;livepatch_hooks_size=%d\n&quot;, size);<br class="">
</blockquote>
<br class="">
If you want to keep this debugging output, rather use log_debug().<br class="">
</div>
</div>
</blockquote>
<div><br class="">
</div>
<div>ACK. Will fix.</div>
<br class="">
<blockquote type="cite" class="">
<div class="">
<div class=""><br class="">
<blockquote type="cite" class="">&#43;<span class="Apple-tab-span" style="white-space:pre">
</span>return size;<br class="">
&nbsp;}<br class="">
&nbsp;&nbsp;&nbsp;/*<br class="">
@@ -1084,6 &#43;1097,14 @@ static struct special_section special_sections[] = {<br class="">
&nbsp;<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"></span>.name<span class="Apple-tab-span" style="white-space:pre">
</span><span class="Apple-tab-span" style="white-space:pre"></span>= &quot;.altinstructions&quot;,<br class="">
&nbsp;<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"></span>.group_size<span class="Apple-tab-span" style="white-space:pre">
</span>= altinstructions_group_size,<br class="">
&nbsp;<span class="Apple-tab-span" style="white-space:pre"> </span>},<br class="">
&#43;<span class="Apple-tab-span" style="white-space:pre"> </span>{<br class="">
&#43;<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"></span>.name<span class="Apple-tab-span" style="white-space:pre">
</span><span class="Apple-tab-span" style="white-space:pre"></span>= &quot;.altinstr_replacement&quot;,<br class="">
&#43;<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"></span>.group_size<span class="Apple-tab-span" style="white-space:pre">
</span>= altinstructions_group_size,<br class="">
&#43;<span class="Apple-tab-span" style="white-space:pre"> </span>},<br class="">
&#43;<span class="Apple-tab-span" style="white-space:pre"> </span>{<br class="">
&#43;<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"></span>.name<span class="Apple-tab-span" style="white-space:pre">
</span><span class="Apple-tab-span" style="white-space:pre"></span>= &quot;.livepatch.hooks&quot;,<br class="">
&#43;<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"></span>.group_size<span class="Apple-tab-span" style="white-space:pre">
</span>= livepatch_hooks_group_size,<br class="">
&#43;<span class="Apple-tab-span" style="white-space:pre"> </span>},<br class="">
&nbsp;<span class="Apple-tab-span" style="white-space:pre"> </span>{},<br class="">
&nbsp;};<br class="">
&nbsp;<br class="">
</blockquote>
<br class="">
I don't understand the point of this change.<br class="">
<br class="">
IIUC unlike .altinstructions, the .altinstr_replacement section does not have a fixed group size, so this change would not work other than by luck. If
</div>
</div>
</blockquote>
<div><br class="">
</div>
<div>That’s true. The .altinstr_replacement group_size should be 0 (aka undefined).</div>
<div><br class="">
</div>
<div>ACK. Will fix.</div>
<br class="">
<blockquote type="cite" class="">
<div class="">
<div class="">the goal was to prune bits of the .altinstr_replacement table that do not need to be included, you need to update the code in kpatch_process_special_sections() which has special handling for .altinstr_replacement. It needs to parse the updated,
 pruned .altinstructions section and regenerate .altinstr_replacement, including only what is needed.<br class="">
<br class="">
</div>
</div>
</blockquote>
<div><br class="">
</div>
<div>No this was not the goal. I am explaining it below.</div>
<br class="">
<blockquote type="cite" class="">
<div class="">
<div class="">I don't know why .livepatch.hooks is included in this list either since all hooks are always included and there is nothing to regenerate/prune.<br class="">
</div>
</div>
</blockquote>
<div><br class="">
</div>
<div>As discussed when reviewing [1], I need a function telling which section is special.</div>
<div>By special I mean:</div>
<div>a) should not be a subject of extra validation (See [2])</div>
<div>b) should not be a subject of STN_UNDEF purging (See [3])</div>
<div>&nbsp;</div>
<div>Thus, I want to explicitly call out all &quot;special&quot; sections by their name and specify</div>
<div>their group_size whenever applicable (I did it incorrectly for .altinstr_replacement,</div>
<div>as you have spotted - thanks!) in the common array and use it with the helper</div>
<div>function or for obtaining a group_size.&nbsp;</div>
<div><br class="">
</div>
[1]&nbsp;<span style="caret-color: rgba(0, 0, 0, 0.85098); color: rgba(0, 0, 0, 0.85098); font-family: &quot;Helvetica Neue&quot;;" class="">[livepatch-build-tools part2 3/6] create-diff-object: Add is_special_section() helper function</span></div>
<span class="">[2]&nbsp;[livepatch-build-tools part3 2/3] create-diff-object: Extend patchability verification: STN_UNDEF<br class="">
</span><span class="">[3]&nbsp;[livepatch-build-tools part3 3/3] create-diff-object: Strip all undefined entires of known size<br class="">
</span><span class=""><br class="">
<blockquote type="cite" class=""><br class="">
Regards,<br class="">
--&nbsp;<br class="">
Ross Lagerwall<br class="">
</blockquote>
<br class="">
</span>
<div><br class="">
</div>
<div>
<div>Best Regards,<br class="">
Pawel Wieczorkiewicz</div>
</div>
<br><br><br>Amazon Development Center Germany GmbH
<br>Krausenstr. 38
<br>10117 Berlin
<br>Geschaeftsfuehrer: Christian Schlaeger, Ralf Herbrich
<br>Ust-ID: DE 289 237 879
<br>Eingetragen am Amtsgericht Charlottenburg HRB 149173 B
<br><br><br>
</body>
</html>
diff mbox series

Patch

diff --git a/create-diff-object.c b/create-diff-object.c
index b0b4dcb..f6060cd 100644
--- a/create-diff-object.c
+++ b/create-diff-object.c
@@ -960,51 +960,64 @@  static void kpatch_mark_constant_labels_same(struct kpatch_elf *kelf)
 
 static int bug_frames_group_size(struct kpatch_elf *kelf, int offset)
 {
-    static int size = 0;
-    char *str;
-    if (!size) {
-        str = getenv("BUG_STRUCT_SIZE");
-        size = str ? atoi(str) : 8;
-    }
-
-    return size;
+	static int size = 0;
+	char *str;
+	if (!size) {
+		str = getenv("BUG_STRUCT_SIZE");
+		size = str ? atoi(str) : 8;
+	}
+
+	return size;
 }
 
 static int bug_frames_3_group_size(struct kpatch_elf *kelf, int offset)
 {
-    static int size = 0;
-    char *str;
-    if (!size) {
-        str = getenv("BUG_STRUCT_SIZE");
-        size = str ? atoi(str) : 16;
-    }
-
-    return size;
+	static int size = 0;
+	char *str;
+	if (!size) {
+		str = getenv("BUG_STRUCT_SIZE");
+		size = str ? atoi(str) : 16;
+	}
+
+	return size;
 }
 
 static int ex_table_group_size(struct kpatch_elf *kelf, int offset)
 {
-    static int size = 0;
-    char *str;
-    if (!size) {
-        str = getenv("EX_STRUCT_SIZE");
-        size = str ? atoi(str) : 8;
-    }
-
-    return size;
+	static int size = 0;
+	char *str;
+	if (!size) {
+		str = getenv("EX_STRUCT_SIZE");
+		size = str ? atoi(str) : 8;
+	}
+
+	return size;
 }
 
 static int altinstructions_group_size(struct kpatch_elf *kelf, int offset)
 {
-    static int size = 0;
-    char *str;
-    if (!size) {
-        str = getenv("ALT_STRUCT_SIZE");
-        size = str ? atoi(str) : 12;
-    }
-
-    printf("altinstr_size=%d\n", size);
-    return size;
+	static int size = 0;
+	char *str;
+	if (!size) {
+		str = getenv("ALT_STRUCT_SIZE");
+		size = str ? atoi(str) : 12;
+	}
+
+	printf("altinstr_size=%d\n", size);
+	return size;
+}
+
+static int livepatch_hooks_group_size(struct kpatch_elf *kelf, int offset)
+{
+	static int size = 0;
+	char *str;
+	if (!size) {
+		str = getenv("HOOK_STRUCT_SIZE");
+		size = str ? atoi(str) : 8;
+	}
+
+	printf("livepatch_hooks_size=%d\n", size);
+	return size;
 }
 
 /*
@@ -1084,6 +1097,14 @@  static struct special_section special_sections[] = {
 		.name		= ".altinstructions",
 		.group_size	= altinstructions_group_size,
 	},
+	{
+		.name		= ".altinstr_replacement",
+		.group_size	= altinstructions_group_size,
+	},
+	{
+		.name		= ".livepatch.hooks",
+		.group_size	= livepatch_hooks_group_size,
+	},
 	{},
 };