RISC-V: Fix the static-PIE non-relocated object check
Checks
Context |
Check |
Description |
redhat-pt-bot/TryBot-apply_patch |
success
|
Patch applied to master at the time it was sent
|
redhat-pt-bot/TryBot-32bit |
success
|
Build for i686
|
linaro-tcwg-bot/tcwg_glibc_build--master-arm |
success
|
Testing passed
|
linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 |
success
|
Testing passed
|
linaro-tcwg-bot/tcwg_glibc_check--master-arm |
success
|
Testing passed
|
linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 |
success
|
Testing passed
|
Commit Message
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(-)
Comments
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.
@@ -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