[PATCH v3] elf: Add ELF_DYNAMIC_AFTER_RELOC to rewrite PLT
H.J. Lu
hjl.tools@gmail.com
Tue Jan 2 17:53:39 GMT 2024
On Tue, Jan 2, 2024 at 9:42 AM Adhemerval Zanella Netto
<adhemerval.zanella@linaro.org> wrote:
>
>
>
> On 02/01/24 14:02, H.J. Lu wrote:
> > On Tue, Jan 2, 2024 at 7:51 AM Adhemerval Zanella Netto
>
> >> Maybe it would be clear to use the value directly:
> >>
> >> if ((uint64_t) disp + UINT64_C (0x80000000) <= UINT64_C (0xffffffff))
> >
> > I was using
> >
> > if ((disp + 0x80000000ULL) <= 0xffffffffULL)
> >
> > before. Should I change it back or with UINT64_C?
>
> I just saw it that you stated it on cover-letter, I don't have a strong opnion
> in fact. So either should be fine.
I will keep it ASIS.
> >
> >> Or (if I am not doing anything wrong):
> >>
> >> if ((uint64_t) dist <= 0x7FFFFFFF)
> >
> > (uint64_t)(INT32_MIN) <= 0x7FFFFFFF
> >
> > is false.
>
> Yeah, I just realize that the update value is a signed one.
>
> >
> >>
> >>> + <= (uint64_t) UINT32_MAX)
> >>> + {
> >>> + /* If the target branch can be reached with a direct branch,
> >>> + rewrite the PLT entry with a direct branch. */
> >>> + if (__glibc_unlikely (GLRO(dl_debug_mask) & DL_DEBUG_BINDINGS))
> >>> + {
> >>> + const char *sym_name = x86_64_reloc_symbol_name (map,
> >>> + reloc);
> >>> + _dl_debug_printf ("changing '%s' PLT entry in '%s' to "
> >>> + "direct branch\n", sym_name,
> >>> + DSO_FILENAME (map->l_name));
> >>> + }
> >>> +
> >>> + pad = branch_start + JMP32_INSN_SIZE;
> >>> +
> >>> + if (__glibc_unlikely (pad > plt_end))
> >>> + {
> >>> + if (__glibc_unlikely (GLRO(dl_debug_mask)
> >>> + & DL_DEBUG_BINDINGS))
> >>> + {
> >>> + const char *sym_name
> >>> + = x86_64_reloc_symbol_name (map, reloc);
> >>> + _dl_debug_printf ("\ninvalid r_addend of "
> >>> + "R_X86_64_JUMP_SLOT against '%s' "
> >>> + "in '%s'\n", sym_name,
> >>> + DSO_FILENAME (map->l_name));
> >>> + }
> >>> +
> >>> + continue;
> >>> + }
> >>> +
> >>> + /* Write out direct branch. */
> >>> + *(uint8_t *) branch_start = JMP32_INSN_OPCODE;
> >>> + *((uint32_t *) (branch_start + 1)) = disp;
> >>
> >> These are technically UB, so maybe use memcpy here:
> >>
> >> memcpy (branch_start, &(uint8_t) { JMP32_INSN_OPCODE}, sizeof (uint8_t));
> >> memcpy (branch_start + 1, &disp, sizeof (uint32_t));
> >
> > It should work since there is no difference from x86-64 memcpy:
> >
> > L(between_8_15):
> > /* From 8 to 15. No branch when size == 8. */
> > movq -8(%rsi, %rdx), %rcx
> > movq (%rsi), %rsi
> > movq %rsi, (%rdi)
> > movq %rcx, -8(%rdi, %rdx)
> > ret
>
> And I would expect that compiler would inline these short memcpy anyway.
If
case R_X86_64_PC32:
value += reloc->r_addend - (ElfW(Addr)) reloc_addr;
*(unsigned int *) reloc_addr = value;
in dl-machine.h works,
*(uint8_t *) branch_start = JMP32_INSN_OPCODE;
*((uint32_t *) (branch_start + 1)) = disp;
must work.
> >
> >>> + }
> >>> + else
> >>> + {
> >>> + if (GL(dl_x86_feature_control).plt_rewrite
> >>> + != plt_rewrite_jmpabs)
> >>> + continue;
> >>> +
> >>> + pad = branch_start + JMPABS_INSN_SIZE;
> >>> +
> >>> + if (pad > plt_end)
> >>> + continue;
> >>> +
> >>> + /* Rewrite the PLT entry with JMPABS. */
> >>> + if (__glibc_unlikely (GLRO(dl_debug_mask) & DL_DEBUG_BINDINGS))
> >>> + {
> >>> + const char *sym_name = x86_64_reloc_symbol_name (map,
> >>> + reloc);
> >>> + _dl_debug_printf ("changing '%s' PLT entry in '%s' to "
> >>> + "JMPABS\n", sym_name,
> >>> + DSO_FILENAME (map->l_name));
> >>> + }
> >>> +
> >>> + /* "jmpabs $target" for 64-bit displacement. NB: JMPABS has
> >>> + a 3-byte opcode + 64bit address. There is a 1-byte overlap
> >>> + between 4-byte write and 8-byte write. */
> >>> + *(uint32_t *) (branch_start) = JMPABS_INSN_OPCODE;
> >>> + *(uint64_t *) (branch_start + 3) = value;
> >>> + }
> >>> +
> >>> + /* Fill the unused part of the PLT entry with INT3. */
> >>> + for (; pad < plt_end; pad++)
> >>> + *(uint8_t *) pad = INT3_INSN_OPCODE;
> >>> + }
> >>> +}
> >>> +
> >>> +static inline void
> >>> +x86_64_rewrite_plt_in_place (struct link_map *map)
> >>> +{
> >>> + /* Adjust DT_X86_64_PLT address and DT_X86_64_PLTSZ values. */
> >>> + ElfW(Addr) plt = (map->l_info[DT_X86_64 (PLT)]->d_un.d_ptr
> >>> + + map->l_addr);
> >>> + size_t pagesize = GLRO(dl_pagesize);
> >>> + ElfW(Addr) plt_aligned = ALIGN_DOWN (plt, pagesize);
> >>> + size_t pltsz = (map->l_info[DT_X86_64 (PLTSZ)]->d_un.d_val
> >>> + + plt - plt_aligned);
> >>> +
> >>> + if (__glibc_unlikely (GLRO(dl_debug_mask) & DL_DEBUG_FILES))
> >>> + _dl_debug_printf ("\nchanging PLT in '%s' to writable\n",
> >>> + DSO_FILENAME (map->l_name));
> >>> +
> >>> + if (__glibc_unlikely (__mprotect ((void *) plt_aligned, pltsz,
> >>> + PROT_WRITE | PROT_READ) < 0))
> >>> + {
> >>> + if (__glibc_unlikely (GLRO(dl_debug_mask) & DL_DEBUG_FILES))
> >>> + _dl_debug_printf ("\nfailed to change PLT in '%s' to writable\n",
> >>> + DSO_FILENAME (map->l_name));
> >>> + return;
> >>> + }
> >>> +
> >>> + x86_64_rewrite_plt (map, plt_aligned, plt_aligned);
> >>> +
> >>> + if (__glibc_unlikely (GLRO(dl_debug_mask) & DL_DEBUG_FILES))
> >>> + _dl_debug_printf ("\nchanging PLT in '%s' back to read-only\n",
> >>> + DSO_FILENAME (map->l_name));
> >>> +
> >>> + if (__glibc_unlikely (__mprotect ((void *) plt_aligned, pltsz,
> >>> + PROT_EXEC | PROT_READ) < 0))
> >>> + _dl_signal_error (0, DSO_FILENAME (map->l_name), NULL,
> >>> + "failed to change PLT back to read-only");
> >>> +}
> >>> +
> >>> +/* Rewrite PLT entries to direct branch if possible. */
> >>> +
> >>> +static inline void
> >>> +x86_64_dynamic_after_reloc (struct link_map *map, int lazy)
> >>> +{
> >>> + /* Ignore DT_X86_64_PLT if the lazy binding is enabled. */
> >>> + if (lazy)
> >>> + return;
> >>> +
> >>> + if (__glibc_likely (map->l_info[DT_X86_64 (PLT)] == NULL))
> >>> + return;
> >>> +
> >>> + /* Ignore DT_X86_64_PLT if there is no R_X86_64_JUMP_SLOT. */
> >>> + if (!map->l_has_jump_slot_reloc)
> >>> + return;
> >>> +
> >>> + /* Ignore DT_X86_64_PLT on ld.so to avoid changing its own PLT. */
> >>> + if (map == &GL(dl_rtld_map) || map->l_real == &GL(dl_rtld_map))
> >>> + return;
> >>> +
> >>> + /* Ignore DT_X86_64_PLT if
> >>> + 1. DT_JMPREL isn't available or its value is 0.
> >>> + 2. DT_PLTRELSZ is 0.
> >>> + 3. DT_X86_64_PLTENT isn't available or its value is smaller than
> >>> + 16 bytes.
> >>> + 4. DT_X86_64_PLTSZ isn't available or its value is smaller than
> >>> + DT_X86_64_PLTENT's value or isn't a multiple of DT_X86_64_PLTENT's
> >>> + value. */
> >>> + if (map->l_info[DT_JMPREL] == NULL
> >>> + || map->l_info[DT_JMPREL]->d_un.d_ptr == 0
> >>> + || map->l_info[DT_PLTRELSZ]->d_un.d_val == 0
> >>> + || map->l_info[DT_X86_64 (PLTSZ)] == NULL
> >>> + || map->l_info[DT_X86_64 (PLTENT)] == NULL
> >>> + || map->l_info[DT_X86_64 (PLTENT)]->d_un.d_val < 16
> >>> + || (map->l_info[DT_X86_64 (PLTSZ)]->d_un.d_val
> >>> + < map->l_info[DT_X86_64 (PLTENT)]->d_un.d_val)
> >>> + || (map->l_info[DT_X86_64 (PLTSZ)]->d_un.d_val
> >>> + % map->l_info[DT_X86_64 (PLTENT)]->d_un.d_val) != 0)
> >>> + return;
> >>> +
> >>> + if (GL(dl_x86_feature_control).plt_rewrite == plt_rewrite_enabled)
> >>> + {
> >>> + enum dl_plt_rewrite_control plt_rewrite = plt_rewrite_none;
> >>> +
> >>> + /* PLT rewrite is enabled. Check if mprotect works. */
> >>> + void *plt = __mmap (NULL, 4096, PROT_READ | PROT_WRITE,
> >>> + MAP_PRIVATE | MAP_ANONYMOUS,
> >>> + -1, 0);
> >>> + if (__glibc_unlikely (plt != MAP_FAILED))
> >>> + {
> >>> + /* Touch the PROT_READ | PROT_WRITE page. */
> >>> + *(int32_t *) plt = -1;
> >>> +
> >>> + /* If the updated PROT_READ | PROT_WRITE page can be changed
> >>> + to PROT_EXEC | PROT_READ, rewrite PLT. */
> >>> + if (__mprotect (plt, 4096, PROT_EXEC | PROT_READ) == 0)
> >>> + /* Use JMPABS on APX processors. */
> >>> + plt_rewrite = (CPU_FEATURE_PRESENT_P (__get_cpu_features (),
> >>> + APX_F)
> >>> + ? plt_rewrite_jmpabs : plt_rewrite_jmp);
> >>> +
> >>> + __munmap (plt, 4096);
> >>> + }
> >>
> >> Do we have x86_64 cpu/kernel that does not support setting PROT_EXEC | PROT_READ
> >> (afaik only i386 has some restrictions)? Because this runtime tests is somewhat
> >> brittle: mmap might fail or being restricted (for instance due filtering or
> >> resource limitation/exaustion).
> >
> > As Florian mentioned, it checks for SELinux restrictions. It isn't 100%
> > foolproof since SELinux may allow it only for non-program mapped memory
> > and disallow it for program mapped memory.
> >
>
> Yeah, I missed the previous conversation about the SELinux issue. I don't have
> a strong preference, but I do tend to see such runtime checks to add some
> maintainability issues specially in this case that they are not really
> bullet-proof.
I can drop it.
--
H.J.
More information about the Libc-alpha
mailing list