Re: [PATCH] Arm32: avoid .rodata to be marked as executable

[Julien Grall <julien@xxxxxxx>]

Hi Jan,

On 14/06/2021 14:02, Jan Beulich wrote:
> On 14.06.2021 12:54, Julien Grall wrote:
> > On 14/06/2021 12:40, Jan Beulich wrote:
> > > On 14.06.2021 11:57, Julien Grall wrote:
> > > > On 11/06/2021 11:19, Jan Beulich wrote:
> > > > > This confuses disassemblers, at the very least. When this data still
> > > > > lived in .init.*, this probably didn't matter much, albeit the
> > > > > "#execinstr" would have been suspicious to me already then. But the
> > > > > latest with their movement to .rodata these attributes should have been
> > > > > dropped.
> > > > I don't quite understand why this wasn't really a problem for .init.data
> > > > but it is a problem for .rodata. Can you expand your thought?
> > > I've said "probably" for a reason, and my thinking here goes along
> > > the lines of what I've said on the other patch regarding .init.*:
> > > There's perhaps not overly much reason to be picky about the
> > > attributes of .init.*, and at least on x86 there is also a case
> > > (the EFI binary) where we fold all .init.* into just .init anyway.
> > Makese sense. Thanks for the explanation.
> >
> > > The alternative to the present description that I see would be to
> > > go with just the 1st sentence. But I would be afraid in such a
> > > case that you would come back and tell me this is too little of a
> > > description.
> > How about:
> >
> > "xen/arm: .proc.info doesn't need to be executable
> >
> > The section .proc.info lives in .rodata as it doesn't contain any
> > executable code. However, the section is still marked as executable as
> > the consequence .rodata will also be marked executable.
> >
> > Xen doesn't use the ELF permissions to decide the page-table mapping
> > permission. However, this will confuse disassemblers.
> >
> > #execinstr is now removed on all the pushsection dealing with .proc.info".
> >
> > I can update the commit message on commit.
> I'm fine with the new commit message, but I'd prefer the title to
> remain as is, as that aspect is what did trigger me making this > change.
Sure. I will keep your commit title and update the commit message.

Cheers,


--
Julien Grall