Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do we need more robust archeticture protection #1356

Open
laokz opened this issue Aug 26, 2023 · 4 comments
Open

Do we need more robust archeticture protection #1356

laokz opened this issue Aug 26, 2023 · 4 comments
Assignees

Comments

@laokz
Copy link

laokz commented Aug 26, 2023

I noticed that there are many places using relocation types without #ifdef architecture protection, such as in create-diff-object.c:

	return (rela_toc && rela_toc->sym->type == STT_FUNC &&
		!rela_toc->sym->parent &&
		rela_toc->addend == (int)rela_toc->sym->sym.st_value &&
		(rela->type == R_X86_64_32S ||
		rela->type == R_PPC64_TOC16_HA ||
		rela->type == R_PPC64_TOC16_LO_DS));

All architectures relocation types use same number space. R_X86_64_32S value is 11. This value for PPC is R_PPC_REL14, for arm is R_ARM_THM_PC8. Could this break the code?

@jpoimboe
Copy link
Member

Good point! We definitely need to fix that.

@joe-lawrence
Copy link
Contributor

Just a heads up, conditionally building code around #ifdef <arch> works against (eventually) cross-building/testing kpatches. I would think the correct thing to do would be the include the source/target along with the relocation enum check.

@github-actions
Copy link

This issue has been open for 30 days with no activity and no assignee. It will be closed in 7 days unless a comment is added.

@github-actions github-actions bot added the stale label Sep 28, 2023
@github-actions
Copy link

github-actions bot commented Oct 5, 2023

This issue was closed because it was inactive for 7 days after being marked stale.

@github-actions github-actions bot closed this as completed Oct 5, 2023
@jpoimboe jpoimboe self-assigned this Oct 5, 2023
@jpoimboe jpoimboe removed the stale label Oct 5, 2023
@jpoimboe jpoimboe reopened this Oct 5, 2023
swine added a commit to swine/kpatch that referenced this issue May 31, 2024
Do we need more robust architecture protection (Issue dynup#1356)

The elf.h reloc-type constants are not unique across archs
  #define R_PPC64_REL24           10      /* PC relative 26 bit */
  #define R_X86_64_32             10      /* Direct 32 bit zero extended */
so to avoid any unexpected aliasing, guard all R_arch_type refs
with a check on kelf->arch, or a global default arch set from
the first elf encountered.
mihails-strasuns pushed a commit to mihails-strasuns/kpatch that referenced this issue Jun 14, 2024
Do we need more robust architecture protection (Issue dynup#1356)

The elf.h reloc-type constants are not unique across archs
  #define R_PPC64_REL24           10      /* PC relative 26 bit */
  #define R_X86_64_32             10      /* Direct 32 bit zero extended */
so to avoid any unexpected aliasing, guard all R_arch_type refs
with a check on kelf->arch, or a global default arch set from
the first elf encountered.
mihails-strasuns pushed a commit to mihails-strasuns/kpatch that referenced this issue Sep 5, 2024
Do we need more robust architecture protection (Issue dynup#1356)

The elf.h reloc-type constants are not unique across archs
  #define R_PPC64_REL24           10      /* PC relative 26 bit */
  #define R_X86_64_32             10      /* Direct 32 bit zero extended */
so to avoid any unexpected aliasing, guard all R_arch_type refs
with a check on kelf->arch, or a global default arch set from
the first elf encountered.
joe-lawrence pushed a commit to joe-lawrence/kpatch that referenced this issue Oct 10, 2024
The elf.h reloc-type constants are not unique across archs, for example:

  #define R_PPC64_REL24           10      /* PC relative 26 bit */
  #define R_X86_64_32             10      /* Direct 32 bit zero extended */

so to avoid any unexpected aliasing, guard all R_arch_type refs with a
check on kelf->arch, or a global default arch set from the first elf
encountered.

Closes: dynup#1356 ("Do we need more robust archeticture protection")
Signed-off-by: Pete Swain <swine@google.com>
Signed-off-by: Mihails Strasuns <mstrasun@amazon.com>
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
joe-lawrence pushed a commit to joe-lawrence/kpatch that referenced this issue Oct 15, 2024
The elf.h reloc-type constants are not unique across archs, for example:

  #define R_PPC64_REL24           10      /* PC relative 26 bit */
  #define R_X86_64_32             10      /* Direct 32 bit zero extended */

so to avoid any unexpected aliasing, guard all R_arch_type refs with a
check on kelf->arch, or a global default arch set from the first elf
encountered.

Closes: dynup#1356 ("Do we need more robust archeticture protection")
Signed-off-by: Pete Swain <swine@google.com>
Signed-off-by: Mihails Strasuns <mstrasun@amazon.com>
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants