| Message ID | 20170609160336.13351-2-ross.lagerwall@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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>
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 >
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
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
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.
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);
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(-)