Bug 26378 - sections initialised only by linker scripts are always read/write
Summary: sections initialised only by linker scripts are always read/write
Status: RESOLVED FIXED
Alias: None
Product: binutils
Classification: Unclassified
Component: ld (show other bugs)
Version: 2.35
: P2 normal
Target Milestone: 2.37
Assignee: Alan Modra
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-08-12 16:33 UTC by Szabolcs Nagy
Modified: 2021-02-26 04:25 UTC (History)
4 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:


Attachments
dubious fix (667 bytes, patch)
2020-09-16 00:13 UTC, Alan Modra
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Szabolcs Nagy 2020-08-12 16:33:17 UTC
since binutils 2.35 i see WAX flags on some nobits sections:

$ cat a.s
.section        .text.foo,"ax",@progbits
.global foo
.type foo,%function
foo:
        ret

$ cat a.ld
SECTIONS {
        .plt (NOLOAD) : { BYTE(0) }
        .text.foo (NOLOAD) : { BYTE(0) }
}

$ as -c -o a.o a.s
$ ld -T a.ld a.o
$ readelf -SW a.out
There are 7 section headers, starting at offset 0x10b0:

Section Headers:
  [Nr] Name              Type            Address          Off    Size   ES Flg Lk Inf Al
  [ 0]                   NULL            0000000000000000 000000 000000 00      0   0  0
  [ 1] .text.foo         PROGBITS        0000000000000000 001000 000001 00  AX  0   0  1
  [ 2] .plt              NOBITS          0000000000000001 001001 000001 00 WAX  0   0  1
  [ 3] .text.foo         NOBITS          0000000000000002 001001 000001 00 WAX  0   0  1
  [ 4] .symtab           SYMTAB          0000000000000000 001008 000078 18      5   4  8
  [ 5] .strtab           STRTAB          0000000000000000 001080 000005 00      0   0  1
  [ 6] .shstrtab         STRTAB          0000000000000000 001085 00002a 00      0   0  1
Key to Flags:
  W (write), A (alloc), X (execute), M (merge), S (strings), I (info),
  L (link order), O (extra OS processing required), G (group), T (TLS),
  C (compressed), x (unknown), o (OS specific), E (exclude),
  l (large), p (processor specific)


if i change '.text.foo' to '.test.foo' in a.s and a.ld
then there is no WAX on that section.

i don't see WAX prior to commit

commit 8c803a2dd7d3d742a3d0071914f557ef465afe71
Commit:     Alan Modra <amodra@gmail.com>
CommitDate: 2020-03-02 11:36:19 +1030

    elf_backend_section_flags and _bfd_elf_init_private_section_data


this affects linux kernel modules on aarch64, but the
behaviour change seems to be target independent.

before the commit above i see:

$ readelf -SW a.out
There are 7 section headers, starting at offset 0x10b0:

Section Headers:
  [Nr] Name              Type            Address          Off    Size   ES Flg Lk Inf Al
  [ 0]                   NULL            0000000000000000 000000 000000 00      0   0  0
  [ 1] .text.foo         PROGBITS        0000000000000000 001000 000001 00  AX  0   0  1
  [ 2] .plt              NOBITS          0000000000000001 001001 000001 00  WA  0   0  1
  [ 3] .text.foo         NOBITS          0000000000000002 001001 000001 00  WA  0   0  1
  [ 4] .symtab           SYMTAB          0000000000000000 001008 000078 18      5   4  8
  [ 5] .strtab           STRTAB          0000000000000000 001080 000005 00      0   0  1
  [ 6] .shstrtab         STRTAB          0000000000000000 001085 00002a 00      0   0  1
Key to Flags:
  W (write), A (alloc), X (execute), M (merge), S (strings), I (info),
  L (link order), O (extra OS processing required), G (group), T (TLS),
  C (compressed), x (unknown), o (OS specific), E (exclude),
  l (large), p (processor specific)
Comment 1 Szabolcs Nagy 2020-08-12 16:48:04 UTC
linux-kernel discussion:
https://lore.kernel.org/lkml/20200812160017.GA30302@linux-8ccs/
Comment 2 H.J. Lu 2020-08-12 17:33:00 UTC
.plt and text.* sections are special sections:

static const struct bfd_elf_special_section special_sections_p[] =
{
  { STRING_COMMA_LEN (".preinit_array"), -2, SHT_PREINIT_ARRAY, SHF_ALLOC + SHF_WRITE },
  { STRING_COMMA_LEN (".plt"),            0, SHT_PROGBITS,      SHF_ALLOC + SHF_EXECINSTR },
  { NULL,                   0,            0, 0,                 0 }
};

static const struct bfd_elf_special_section special_sections_t[] =
{
  { STRING_COMMA_LEN (".text"),  -2, SHT_PROGBITS, SHF_ALLOC + SHF_EXECINSTR },
  { STRING_COMMA_LEN (".tbss"),  -2, SHT_NOBITS,   SHF_ALLOC + SHF_WRITE + SHF_TLS },
  { STRING_COMMA_LEN (".tdata"), -2, SHT_PROGBITS, SHF_ALLOC + SHF_WRITE + SHF_TLS },
  { NULL,                     0,  0, 0,            0 }
};

They have the SHF_EXECINSTR bit set.  Maybe we should also check if the type
field matches.
Comment 3 H.J. Lu 2020-08-12 19:04:27 UTC
Why is WAX a problem for kernel modules?
Comment 4 H.J. Lu 2020-08-12 19:05:17 UTC
The relevant code is in assign_file_positions_for_load_sections:

         for (i = 0; i < m->count; i++)
            if ((m->sections[i]->flags & (SEC_LOAD | SEC_HAS_CONTENTS)) == 0) 
              /* If we aren't making room for this section, then
                 it must be SHT_NOBITS regardless of what we've
                 set via struct bfd_elf_special_section.  */
              elf_section_type (m->sections[i]) = SHT_NOBITS;
Comment 5 Alan Modra 2020-08-13 00:16:58 UTC
(In reply to Szabolcs Nagy from comment #0)
> i don't see WAX prior to commit
> 
> commit 8c803a2dd7d3d742a3d0071914f557ef465afe71

Yes, that'a a bug fix.  A missing X on .plt or .text.* is a bug.
Comment 6 Fangrui Song 2020-09-05 22:29:46 UTC
(In reply to Alan Modra from comment #5)
> (In reply to Szabolcs Nagy from comment #0)
> > i don't see WAX prior to commit
> > 
> > commit 8c803a2dd7d3d742a3d0071914f557ef465afe71
> 
> Yes, that'a a bug fix.  A missing X on .plt or .text.* is a bug.

Why does .plt have the SHF_WRITE flag? `BYTE(0)`? `. += 1;`?

There would not be a problem if SHF_WRITE were not added.
Comment 7 Alan Modra 2020-09-11 13:17:29 UTC
Your script made it NOLOAD, which for ELF means a bss style section.
Comment 8 Fangrui Song 2020-09-11 16:05:44 UTC
(In reply to Alan Modra from comment #7)
> Your script made it NOLOAD, which for ELF means a bss style section.

I think the only hard requirement is the section type: SHT_NOBITS, not the section flag: SHF_WRITE.

The documentation raises an example with a section named 'ROM': https://sourceware.org/binutils/docs/ld/Output-Section-Type.html
One would not consider 'ROM' writable.
Comment 9 Alan Modra 2020-09-12 00:04:29 UTC
Yes, noload behaviour does not match the documentation.  Worse, noload has evolved to mean different things depending on object file format.  See https://sourceware.org/pipermail/binutils/2010-September/068723.html and other messages in that and following months with the subject "SEC_NEVER_LOAD cleanup".

As far as the SHF_WRITE flag goes, that is of course the normal thing for bss style sections.  I do understand why you might want a bss style .plt read-only (with ld.so flipping page protection bits to write and initialise the contents) so I'm not saying ld can't change, just explaining the current situation.
Comment 10 Alan Modra 2020-09-16 00:13:42 UTC
Created attachment 12844 [details]
dubious fix

We could change the default for sections with input only from linker scripts, but that will almost certainly break something for someone.
Comment 11 Sourceware Commits 2021-01-18 12:39:33 UTC
The master branch has been updated by Alan Modra <amodra@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=326adec374dd43086dbf9bb2b8f18d547389e678

commit 326adec374dd43086dbf9bb2b8f18d547389e678
Author: Alan Modra <amodra@gmail.com>
Date:   Sat Sep 12 10:49:13 2020 +0930

    PR26378, sections initialised only by linker scripts are always read/write
    
    This changes the initialisation of output sections so that it is
    possible to create read-only sections fed only from linker script
    BYTE, SHORT, LONG or QUAD.  That currently isn't possible even for one
    of the well-known ELF sections like .rodata, because once a section is
    marked read/write that sticks.  On the other hand if we start
    read-only, well-known ELF sections end up read/write as appropriate.
    For example .tdata will still be SHF_ALLOC + SHF_WRITE + SHF_TLS.
    
            PR 26378
            * ldlang.c (map_input_to_output_sections): Start with a read-only
            section for data statements.
            * testsuite/ld-elf/size-2.d: Adjust to suit.