[PATCH LIVEPATCH-BUILD-TOOLS 1/2] Ignore .discard sections

[PATCH LIVEPATCH-BUILD-TOOLS 1/2] Ignore .discard sections

[Ross Lagerwall]

Ignore differences in discard sections. They are not included in the final xen
binary so there is no need to include them in the live patch.

Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
---
 create-diff-object.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/create-diff-object.c b/create-diff-object.c
index 217e58f..ba19daf 100644
--- a/create-diff-object.c
+++ b/create-diff-object.c
@@ -888,6 +888,14 @@ static void kpatch_mark_ignored_sections(struct kpatch_elf *kelf)
 	struct rela *rela;
 	char *name;
 
+	/* Always ignore .discard sections */
+	sec = find_section_by_name(&kelf->sections, ".discard");
+	if (sec) {
+		sec->ignore = 1;
+		if (sec->twin)
+			sec->twin->ignore = 1;
+	}
+
 	sec = find_section_by_name(&kelf->sections, ".livepatch.ignore.sections");
 	if (!sec)
 		return;
-- 
2.9.4


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

[PATCH LIVEPATCH-BUILD-TOOLS 2/2] Remove section alignment requirement

[Ross Lagerwall]

Remove the requirement that section twins have the same alignment. The
section alignment of the patched section is respected by the loader in
Xen so it shouldn't matter if the original section alignment was
different.

Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
---
 create-diff-object.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/create-diff-object.c b/create-diff-object.c
index ba19daf..82f777e 100644
--- a/create-diff-object.c
+++ b/create-diff-object.c
@@ -754,7 +754,6 @@ static void kpatch_compare_correlated_section(struct section *sec)
 	if (sec1->sh.sh_type != sec2->sh.sh_type ||
 	    sec1->sh.sh_flags != sec2->sh.sh_flags ||
 	    sec1->sh.sh_addr != sec2->sh.sh_addr ||
-	    sec1->sh.sh_addralign != sec2->sh.sh_addralign ||
 	    sec1->sh.sh_entsize != sec2->sh.sh_entsize)
 		DIFF_FATAL("%s section header details differ", sec1->name);
 
-- 
2.9.4


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

Re: [PATCH LIVEPATCH-BUILD-TOOLS 2/2] Remove section alignment requirement

[Andrew Cooper]

On 09/06/17 17:03, Ross Lagerwall wrote:
> Remove the requirement that section twins have the same alignment. The
> section alignment of the patched section is respected by the loader in
> Xen so it shouldn't matter if the original section alignment was
> different.
>
> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

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

Re: [PATCH LIVEPATCH-BUILD-TOOLS 2/2] Remove section alignment requirement

[Konrad Rzeszutek Wilk]

On Fri, Jun 09, 2017 at 05:03:36PM +0100, Ross Lagerwall wrote:
> Remove the requirement that section twins have the same alignment. The
> section alignment of the patched section is respected by the loader in
> Xen so it shouldn't matter if the original section alignment was
> different.

Why would we have different section aligment for the same twins?

I understand the change here - if for example the original code
had:

 .rodata.str.1

[I think I got the section name right - that should be a string
generated with 1 byte alignment, say 'a'].

But the new patch has a new one:

 .rodata.str.2

['a' -> 'ab']

Is that what we are fixing here?

Also CC-ing Sarah
> 
> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
> ---
>  create-diff-object.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/create-diff-object.c b/create-diff-object.c
> index ba19daf..82f777e 100644
> --- a/create-diff-object.c
> +++ b/create-diff-object.c
> @@ -754,7 +754,6 @@ static void kpatch_compare_correlated_section(struct section *sec)
>  	if (sec1->sh.sh_type != sec2->sh.sh_type ||
>  	    sec1->sh.sh_flags != sec2->sh.sh_flags ||
>  	    sec1->sh.sh_addr != sec2->sh.sh_addr ||
> -	    sec1->sh.sh_addralign != sec2->sh.sh_addralign ||
>  	    sec1->sh.sh_entsize != sec2->sh.sh_entsize)
>  		DIFF_FATAL("%s section header details differ", sec1->name);
>  
> -- 
> 2.9.4
> 

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

Re: [PATCH LIVEPATCH-BUILD-TOOLS 2/2] Remove section alignment requirement

[Andrew Cooper]

On 09/06/17 17:38, Konrad Rzeszutek Wilk wrote:
> On Fri, Jun 09, 2017 at 05:03:36PM +0100, Ross Lagerwall wrote:
>> Remove the requirement that section twins have the same alignment. The
>> section alignment of the patched section is respected by the loader in
>> Xen so it shouldn't matter if the original section alignment was
>> different.
> Why would we have different section aligment for the same twins?
>
> I understand the change here - if for example the original code
> had:
>
>  .rodata.str.1
>
> [I think I got the section name right - that should be a string
> generated with 1 byte alignment, say 'a'].
>
> But the new patch has a new one:
>
>  .rodata.str.2
>
> ['a' -> 'ab']
>
> Is that what we are fixing here?

Functions also have alignment.  For reasons best known to the compiler,
the fix for XSA-213 caused the alignment of arch_do_multicall_call() to
change from 1 to 16.

The build tools choked on this, but the result is legitimate, and did
work fine.

~Andrew

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

Re: [PATCH LIVEPATCH-BUILD-TOOLS 2/2] Remove section alignment requirement

[Konrad Rzeszutek Wilk]

On Fri, Jun 09, 2017 at 06:00:35PM +0100, Andrew Cooper wrote:
> On 09/06/17 17:38, Konrad Rzeszutek Wilk wrote:
> > On Fri, Jun 09, 2017 at 05:03:36PM +0100, Ross Lagerwall wrote:
> >> Remove the requirement that section twins have the same alignment. The
> >> section alignment of the patched section is respected by the loader in
> >> Xen so it shouldn't matter if the original section alignment was
> >> different.
> > Why would we have different section aligment for the same twins?
> >
> > I understand the change here - if for example the original code
> > had:
> >
> >  .rodata.str.1
> >
> > [I think I got the section name right - that should be a string
> > generated with 1 byte alignment, say 'a'].
> >
> > But the new patch has a new one:
> >
> >  .rodata.str.2
> >
> > ['a' -> 'ab']
> >
> > Is that what we are fixing here?
> 
> Functions also have alignment.  For reasons best known to the compiler,
> the fix for XSA-213 caused the alignment of arch_do_multicall_call() to
> change from 1 to 16.

OK, could this be please be included in the commit description?

And with Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

Ross,  you ok committing it in livepatch-build-tools.git? Thanks!
> 
> The build tools choked on this, but the result is legitimate, and did
> work fine.
> 
> ~Andrew

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

Re: [PATCH LIVEPATCH-BUILD-TOOLS 2/2] Remove section alignment requirement

[Ross Lagerwall]

On 06/09/2017 06:08 PM, Konrad Rzeszutek Wilk wrote:
> On Fri, Jun 09, 2017 at 06:00:35PM +0100, Andrew Cooper wrote:
>> On 09/06/17 17:38, Konrad Rzeszutek Wilk wrote:
>>> On Fri, Jun 09, 2017 at 05:03:36PM +0100, Ross Lagerwall wrote:
>>>> Remove the requirement that section twins have the same alignment. The
>>>> section alignment of the patched section is respected by the loader in
>>>> Xen so it shouldn't matter if the original section alignment was
>>>> different.
>>> Why would we have different section aligment for the same twins?
>>>
>>> I understand the change here - if for example the original code
>>> had:
>>>
>>>   .rodata.str.1
>>>
>>> [I think I got the section name right - that should be a string
>>> generated with 1 byte alignment, say 'a'].
>>>
>>> But the new patch has a new one:
>>>
>>>   .rodata.str.2
>>>
>>> ['a' -> 'ab']
>>>
>>> Is that what we are fixing here?
>>
>> Functions also have alignment.  For reasons best known to the compiler,
>> the fix for XSA-213 caused the alignment of arch_do_multicall_call() to
>> change from 1 to 16.
> 
> OK, could this be please be included in the commit description?

Done.

> 
> And with Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> 
> Ross,  you ok committing it in livepatch-build-tools.git? Thanks!
>>

Done.

Thanks for reporting and reviewing.

-- 
Ross Lagerwall

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

Re: [PATCH LIVEPATCH-BUILD-TOOLS 1/2] Ignore .discard sections

[Andrew Cooper]

On 09/06/17 17:03, Ross Lagerwall wrote:
> Ignore differences in discard sections. They are not included in the final xen
> binary so there is no need to include them in the live patch.
>
> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

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

Re: [PATCH LIVEPATCH-BUILD-TOOLS 1/2] Ignore .discard sections

[Konrad Rzeszutek Wilk]

On Fri, Jun 09, 2017 at 05:03:35PM +0100, Ross Lagerwall wrote:
> Ignore differences in discard sections. They are not included in the final xen
> binary so there is no need to include them in the live patch.
> 
> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>

Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

(And CC-ing Sarah)
> ---
>  create-diff-object.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/create-diff-object.c b/create-diff-object.c
> index 217e58f..ba19daf 100644
> --- a/create-diff-object.c
> +++ b/create-diff-object.c
> @@ -888,6 +888,14 @@ static void kpatch_mark_ignored_sections(struct kpatch_elf *kelf)
>  	struct rela *rela;
>  	char *name;
>  
> +	/* Always ignore .discard sections */
> +	sec = find_section_by_name(&kelf->sections, ".discard");
> +	if (sec) {
> +		sec->ignore = 1;
> +		if (sec->twin)
> +			sec->twin->ignore = 1;
> +	}
> +
>  	sec = find_section_by_name(&kelf->sections, ".livepatch.ignore.sections");
>  	if (!sec)
>  		return;
> -- 
> 2.9.4
> 

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