Message ID | 20240222232400.6326-1-palmer@rivosinc.com |
---|---|
State | New |
Headers | show |
Series | RISC-V: Fix the static-PIE non-relocated object check | expand |
Hi Palmer, Thanks for your patch. I have did the full tests with the same change as this patch on my qemu-system. There's no regression introduced. Also test this change with mold linker. It passed. Thanks, Yanzhang > -----Original Message----- > From: Palmer Dabbelt <palmer@rivosinc.com> > Sent: Friday, February 23, 2024 7:24 AM > To: libc-alpha@sourceware.org > Cc: schwab@suse.de; Wang, Yanzhang <yanzhang.wang@intel.com>; > adhemerval.zanella@linaro.org; Palmer Dabbelt <palmer@rivosinc.com> > Subject: [PATCH] RISC-V: Fix the static-PIE non-relocated object check > > The value of l_scope is only valid post relocation, so this original check > was triggering undefined behavior. Instead just directly check to see if > the object has been relocated, at which point using l_scope is safe. > > Reported-by: Andreas Schwab <schwab@suse.de> > Closes: BZ #31317 > Fixes: e0590f41fe ("RISC-V: Enable static-pie.") > Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com> > --- > sysdeps/riscv/dl-machine.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/sysdeps/riscv/dl-machine.h b/sysdeps/riscv/dl-machine.h index > 0cbb476c05..b2f28697f7 100644 > --- a/sysdeps/riscv/dl-machine.h > +++ b/sysdeps/riscv/dl-machine.h > @@ -348,7 +348,7 @@ elf_machine_runtime_setup (struct link_map *l, struct > r_scope_elem *scope[], > gotplt[1] = (ElfW(Addr)) l; > } > > - if (l->l_type == lt_executable && l->l_scope != NULL) > + if (l->l_type == lt_executable && l->l_relocated) > { > /* The __global_pointer$ may not be defined by the linker if the > $gp register does not be used to access the global variable > -- > 2.43.0
On Thu, 22 Feb 2024 18:16:47 PST (-0800), yanzhang.wang@intel.com wrote: > Hi Palmer, > > Thanks for your patch. I have did the full tests with the same change > as this patch on my qemu-system. There's no regression introduced. > > Also test this change with mold linker. It passed. Awesome, thanks. I don't have a good setup for this so I was sort of just trying to figure it out from poking aroud the code and whatever Andres said. Are you OK posting a Tested-by? This will probably get backported, so best to get that stuff sorted out. > > Thanks, > Yanzhang > >> -----Original Message----- >> From: Palmer Dabbelt <palmer@rivosinc.com> >> Sent: Friday, February 23, 2024 7:24 AM >> To: libc-alpha@sourceware.org >> Cc: schwab@suse.de; Wang, Yanzhang <yanzhang.wang@intel.com>; >> adhemerval.zanella@linaro.org; Palmer Dabbelt <palmer@rivosinc.com> >> Subject: [PATCH] RISC-V: Fix the static-PIE non-relocated object check >> >> The value of l_scope is only valid post relocation, so this original check >> was triggering undefined behavior. Instead just directly check to see if >> the object has been relocated, at which point using l_scope is safe. >> >> Reported-by: Andreas Schwab <schwab@suse.de> >> Closes: BZ #31317 >> Fixes: e0590f41fe ("RISC-V: Enable static-pie.") >> Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com> >> --- >> sysdeps/riscv/dl-machine.h | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/sysdeps/riscv/dl-machine.h b/sysdeps/riscv/dl-machine.h index >> 0cbb476c05..b2f28697f7 100644 >> --- a/sysdeps/riscv/dl-machine.h >> +++ b/sysdeps/riscv/dl-machine.h >> @@ -348,7 +348,7 @@ elf_machine_runtime_setup (struct link_map *l, struct >> r_scope_elem *scope[], >> gotplt[1] = (ElfW(Addr)) l; >> } >> >> - if (l->l_type == lt_executable && l->l_scope != NULL) >> + if (l->l_type == lt_executable && l->l_relocated) >> { >> /* The __global_pointer$ may not be defined by the linker if the >> $gp register does not be used to access the global variable >> -- >> 2.43.0
OK, no problem. > -----Original Message----- > From: Palmer Dabbelt <palmer@rivosinc.com> > Sent: Friday, February 23, 2024 11:07 AM > To: Wang, Yanzhang <yanzhang.wang@intel.com> > Cc: libc-alpha@sourceware.org; schwab@suse.de; > adhemerval.zanella@linaro.org > Subject: RE: [PATCH] RISC-V: Fix the static-PIE non-relocated object check > > On Thu, 22 Feb 2024 18:16:47 PST (-0800), yanzhang.wang@intel.com wrote: > > Hi Palmer, > > > > Thanks for your patch. I have did the full tests with the same change > > as this patch on my qemu-system. There's no regression introduced. > > > > Also test this change with mold linker. It passed. > > Awesome, thanks. I don't have a good setup for this so I was sort of just > trying to figure it out from poking aroud the code and whatever Andres said. > > Are you OK posting a Tested-by? This will probably get backported, so best > to get that stuff sorted out. > > > > > Thanks, > > Yanzhang > > > >> -----Original Message----- > >> From: Palmer Dabbelt <palmer@rivosinc.com> > >> Sent: Friday, February 23, 2024 7:24 AM > >> To: libc-alpha@sourceware.org > >> Cc: schwab@suse.de; Wang, Yanzhang <yanzhang.wang@intel.com>; > >> adhemerval.zanella@linaro.org; Palmer Dabbelt <palmer@rivosinc.com> > >> Subject: [PATCH] RISC-V: Fix the static-PIE non-relocated object > >> check > >> > >> The value of l_scope is only valid post relocation, so this original > >> check was triggering undefined behavior. Instead just directly check > >> to see if the object has been relocated, at which point using l_scope is > safe. > >> > >> Reported-by: Andreas Schwab <schwab@suse.de> > >> Closes: BZ #31317 > >> Fixes: e0590f41fe ("RISC-V: Enable static-pie.") > >> Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com> > >> --- > >> sysdeps/riscv/dl-machine.h | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/sysdeps/riscv/dl-machine.h b/sysdeps/riscv/dl-machine.h > >> index > >> 0cbb476c05..b2f28697f7 100644 > >> --- a/sysdeps/riscv/dl-machine.h > >> +++ b/sysdeps/riscv/dl-machine.h > >> @@ -348,7 +348,7 @@ elf_machine_runtime_setup (struct link_map *l, > >> struct r_scope_elem *scope[], > >> gotplt[1] = (ElfW(Addr)) l; > >> } > >> > >> - if (l->l_type == lt_executable && l->l_scope != NULL) > >> + if (l->l_type == lt_executable && l->l_relocated) > >> { > >> /* The __global_pointer$ may not be defined by the linker if the > >> $gp register does not be used to access the global variable > >> -- > >> 2.43.0
On Feb 22 2024, Palmer Dabbelt wrote: > The value of l_scope is only valid post relocation, so this original > check was triggering undefined behavior. Instead just directly check to > see if the object has been relocated, at which point using l_scope is > safe. Ok.
diff --git a/sysdeps/riscv/dl-machine.h b/sysdeps/riscv/dl-machine.h index 0cbb476c05..b2f28697f7 100644 --- a/sysdeps/riscv/dl-machine.h +++ b/sysdeps/riscv/dl-machine.h @@ -348,7 +348,7 @@ elf_machine_runtime_setup (struct link_map *l, struct r_scope_elem *scope[], gotplt[1] = (ElfW(Addr)) l; } - if (l->l_type == lt_executable && l->l_scope != NULL) + if (l->l_type == lt_executable && l->l_relocated) { /* The __global_pointer$ may not be defined by the linker if the $gp register does not be used to access the global variable
The value of l_scope is only valid post relocation, so this original check was triggering undefined behavior. Instead just directly check to see if the object has been relocated, at which point using l_scope is safe. Reported-by: Andreas Schwab <schwab@suse.de> Closes: BZ #31317 Fixes: e0590f41fe ("RISC-V: Enable static-pie.") Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com> --- sysdeps/riscv/dl-machine.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)