* [PATCH] elf: Fix rtld-audit trampoline for aarch64
@ 2022-11-17 8:47 Vladislav Khmelevsky via Libc-alpha
2022-11-17 18:15 ` Adhemerval Zanella Netto via Libc-alpha
0 siblings, 1 reply; 6+ messages in thread
From: Vladislav Khmelevsky via Libc-alpha @ 2022-11-17 8:47 UTC (permalink / raw)
To: libc-alpha; +Cc: woodard, adhemerval.zanella, Vladislav Khmelevsky
This patch fixes two problems with audit:
1. The DL_OFFSET_RV_VPCS offset was mixed up with DL_OFFSET_RG_VPCS,
resulting in x2 register value nulling in RG structure.
2. We need to preserve the x8 register before function call, but don't have
to save it's new value and restore it before return. Anyway the final
restore was using OFFSET_RV instead of OFFSET_RG value which is wrong (althoug doesn't affect anything).
---
sysdeps/aarch64/dl-trampoline.S | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/sysdeps/aarch64/dl-trampoline.S b/sysdeps/aarch64/dl-trampoline.S
index 909b208578..d66f0b9c45 100644
--- a/sysdeps/aarch64/dl-trampoline.S
+++ b/sysdeps/aarch64/dl-trampoline.S
@@ -298,12 +298,11 @@ _dl_runtime_profile:
stp x2, x3, [x29, #OFFSET_RV + DL_OFFSET_RV_X0 + 16*1]
stp x4, x5, [x29, #OFFSET_RV + DL_OFFSET_RV_X0 + 16*2]
stp x6, x7, [x29, #OFFSET_RV + DL_OFFSET_RV_X0 + 16*3]
- str x8, [x29, #OFFSET_RG + DL_OFFSET_RG_X0 + 16*4]
stp q0, q1, [x29, #OFFSET_RV + DL_OFFSET_RV_V0 + 32*0]
stp q2, q3, [x29, #OFFSET_RV + DL_OFFSET_RV_V0 + 32*1]
stp q4, q5, [x29, #OFFSET_RV + DL_OFFSET_RV_V0 + 32*2]
stp q6, q7, [x29, #OFFSET_RV + DL_OFFSET_RV_V0 + 32*3]
- str xzr, [X29, #OFFSET_RV + DL_OFFSET_RG_VPCS]
+ str xzr, [X29, #OFFSET_RV + DL_OFFSET_RV_VPCS]
/* Setup call to pltexit */
ldp x0, x1, [x29, #OFFSET_SAVED_CALL_X0]
@@ -315,7 +314,6 @@ _dl_runtime_profile:
ldp x2, x3, [x29, #OFFSET_RV + DL_OFFSET_RV_X0 + 16*1]
ldp x4, x5, [x29, #OFFSET_RV + DL_OFFSET_RV_X0 + 16*2]
ldp x6, x7, [x29, #OFFSET_RV + DL_OFFSET_RV_X0 + 16*3]
- ldr x8, [x29, #OFFSET_RV + DL_OFFSET_RV_X0 + 16*4]
ldp q0, q1, [x29, #OFFSET_RV + DL_OFFSET_RV_V0 + 32*0]
ldp q2, q3, [x29, #OFFSET_RV + DL_OFFSET_RV_V0 + 32*1]
ldp q4, q5, [x29, #OFFSET_RV + DL_OFFSET_RV_V0 + 32*2]
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] elf: Fix rtld-audit trampoline for aarch64
2022-11-17 8:47 [PATCH] elf: Fix rtld-audit trampoline for aarch64 Vladislav Khmelevsky via Libc-alpha
@ 2022-11-17 18:15 ` Adhemerval Zanella Netto via Libc-alpha
2022-11-17 18:23 ` Vladislav Khmelevsky via Libc-alpha
0 siblings, 1 reply; 6+ messages in thread
From: Adhemerval Zanella Netto via Libc-alpha @ 2022-11-17 18:15 UTC (permalink / raw)
To: Vladislav Khmelevsky, libc-alpha; +Cc: woodard
On 17/11/22 05:47, Vladislav Khmelevsky wrote:
> This patch fixes two problems with audit:
> 1. The DL_OFFSET_RV_VPCS offset was mixed up with DL_OFFSET_RG_VPCS,
> resulting in x2 register value nulling in RG structure.
> 2. We need to preserve the x8 register before function call, but don't have
> to save it's new value and restore it before return. Anyway the final
> restore was using OFFSET_RV instead of OFFSET_RG value which is wrong (althoug doesn't affect anything).
Patch looks ok, although I think currently the ABI only uses x0 and/or x1
to return value (for __int128_t for instance). So I think it should not
be a user-visible issue (at least tst-audit26 does check that lr_vpcs
and lrv_vpcs are zeroed). Are you seeing any issue with current code?
If so could you open a bug please?
> ---
> sysdeps/aarch64/dl-trampoline.S | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/sysdeps/aarch64/dl-trampoline.S b/sysdeps/aarch64/dl-trampoline.S
> index 909b208578..d66f0b9c45 100644
> --- a/sysdeps/aarch64/dl-trampoline.S
> +++ b/sysdeps/aarch64/dl-trampoline.S
> @@ -298,12 +298,11 @@ _dl_runtime_profile:
> stp x2, x3, [x29, #OFFSET_RV + DL_OFFSET_RV_X0 + 16*1]
> stp x4, x5, [x29, #OFFSET_RV + DL_OFFSET_RV_X0 + 16*2]
> stp x6, x7, [x29, #OFFSET_RV + DL_OFFSET_RV_X0 + 16*3]
> - str x8, [x29, #OFFSET_RG + DL_OFFSET_RG_X0 + 16*4]
> stp q0, q1, [x29, #OFFSET_RV + DL_OFFSET_RV_V0 + 32*0]
> stp q2, q3, [x29, #OFFSET_RV + DL_OFFSET_RV_V0 + 32*1]
> stp q4, q5, [x29, #OFFSET_RV + DL_OFFSET_RV_V0 + 32*2]
> stp q6, q7, [x29, #OFFSET_RV + DL_OFFSET_RV_V0 + 32*3]
> - str xzr, [X29, #OFFSET_RV + DL_OFFSET_RG_VPCS]
> + str xzr, [X29, #OFFSET_RV + DL_OFFSET_RV_VPCS]
>
> /* Setup call to pltexit */
> ldp x0, x1, [x29, #OFFSET_SAVED_CALL_X0]
> @@ -315,7 +314,6 @@ _dl_runtime_profile:
> ldp x2, x3, [x29, #OFFSET_RV + DL_OFFSET_RV_X0 + 16*1]
> ldp x4, x5, [x29, #OFFSET_RV + DL_OFFSET_RV_X0 + 16*2]
> ldp x6, x7, [x29, #OFFSET_RV + DL_OFFSET_RV_X0 + 16*3]
> - ldr x8, [x29, #OFFSET_RV + DL_OFFSET_RV_X0 + 16*4]
> ldp q0, q1, [x29, #OFFSET_RV + DL_OFFSET_RV_V0 + 32*0]
> ldp q2, q3, [x29, #OFFSET_RV + DL_OFFSET_RV_V0 + 32*1]
> ldp q4, q5, [x29, #OFFSET_RV + DL_OFFSET_RV_V0 + 32*2]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] elf: Fix rtld-audit trampoline for aarch64
2022-11-17 18:15 ` Adhemerval Zanella Netto via Libc-alpha
@ 2022-11-17 18:23 ` Vladislav Khmelevsky via Libc-alpha
2022-11-17 18:36 ` Adhemerval Zanella Netto via Libc-alpha
0 siblings, 1 reply; 6+ messages in thread
From: Vladislav Khmelevsky via Libc-alpha @ 2022-11-17 18:23 UTC (permalink / raw)
To: Adhemerval Zanella Netto; +Cc: libc-alpha, woodard
Thank you for reviewing!
Yes, it is true tat inly x0/x1 are used as return register. But I have a specific audit library code that was storing some metadata in a free registers during plt entrer and reading it during plt exit :) As for a normal use cases both problems doesn't really affect anything.
> 17 нояб. 2022 г., в 22:15, Adhemerval Zanella Netto <adhemerval.zanella@linaro.org> написал(а):
>
>
>
> On 17/11/22 05:47, Vladislav Khmelevsky wrote:
>> This patch fixes two problems with audit:
>> 1. The DL_OFFSET_RV_VPCS offset was mixed up with DL_OFFSET_RG_VPCS,
>> resulting in x2 register value nulling in RG structure.
>> 2. We need to preserve the x8 register before function call, but don't have
>> to save it's new value and restore it before return. Anyway the final
>> restore was using OFFSET_RV instead of OFFSET_RG value which is wrong (althoug doesn't affect anything).
>
> Patch looks ok, although I think currently the ABI only uses x0 and/or x1
> to return value (for __int128_t for instance). So I think it should not
> be a user-visible issue (at least tst-audit26 does check that lr_vpcs
> and lrv_vpcs are zeroed). Are you seeing any issue with current code?
> If so could you open a bug please?
>
>> ---
>> sysdeps/aarch64/dl-trampoline.S | 4 +---
>> 1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/sysdeps/aarch64/dl-trampoline.S b/sysdeps/aarch64/dl-trampoline.S
>> index 909b208578..d66f0b9c45 100644
>> --- a/sysdeps/aarch64/dl-trampoline.S
>> +++ b/sysdeps/aarch64/dl-trampoline.S
>> @@ -298,12 +298,11 @@ _dl_runtime_profile:
>> stp x2, x3, [x29, #OFFSET_RV + DL_OFFSET_RV_X0 + 16*1]
>> stp x4, x5, [x29, #OFFSET_RV + DL_OFFSET_RV_X0 + 16*2]
>> stp x6, x7, [x29, #OFFSET_RV + DL_OFFSET_RV_X0 + 16*3]
>> - str x8, [x29, #OFFSET_RG + DL_OFFSET_RG_X0 + 16*4]
>> stp q0, q1, [x29, #OFFSET_RV + DL_OFFSET_RV_V0 + 32*0]
>> stp q2, q3, [x29, #OFFSET_RV + DL_OFFSET_RV_V0 + 32*1]
>> stp q4, q5, [x29, #OFFSET_RV + DL_OFFSET_RV_V0 + 32*2]
>> stp q6, q7, [x29, #OFFSET_RV + DL_OFFSET_RV_V0 + 32*3]
>> - str xzr, [X29, #OFFSET_RV + DL_OFFSET_RG_VPCS]
>> + str xzr, [X29, #OFFSET_RV + DL_OFFSET_RV_VPCS]
>>
>> /* Setup call to pltexit */
>> ldp x0, x1, [x29, #OFFSET_SAVED_CALL_X0]
>> @@ -315,7 +314,6 @@ _dl_runtime_profile:
>> ldp x2, x3, [x29, #OFFSET_RV + DL_OFFSET_RV_X0 + 16*1]
>> ldp x4, x5, [x29, #OFFSET_RV + DL_OFFSET_RV_X0 + 16*2]
>> ldp x6, x7, [x29, #OFFSET_RV + DL_OFFSET_RV_X0 + 16*3]
>> - ldr x8, [x29, #OFFSET_RV + DL_OFFSET_RV_X0 + 16*4]
>> ldp q0, q1, [x29, #OFFSET_RV + DL_OFFSET_RV_V0 + 32*0]
>> ldp q2, q3, [x29, #OFFSET_RV + DL_OFFSET_RV_V0 + 32*1]
>> ldp q4, q5, [x29, #OFFSET_RV + DL_OFFSET_RV_V0 + 32*2]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] elf: Fix rtld-audit trampoline for aarch64
2022-11-17 18:23 ` Vladislav Khmelevsky via Libc-alpha
@ 2022-11-17 18:36 ` Adhemerval Zanella Netto via Libc-alpha
2022-11-17 18:51 ` Vladislav Khmelevsky via Libc-alpha
0 siblings, 1 reply; 6+ messages in thread
From: Adhemerval Zanella Netto via Libc-alpha @ 2022-11-17 18:36 UTC (permalink / raw)
To: Vladislav Khmelevsky; +Cc: libc-alpha, woodard
On 17/11/22 15:23, Vladislav Khmelevsky wrote:
> Thank you for reviewing!
> Yes, it is true tat inly x0/x1 are used as return register. But I have a specific audit library code that was storing some metadata in a free registers during plt entrer and reading it during plt exit :) As for a normal use cases both problems doesn't really affect anything.
>
Fair enough, although we might want to backport this.
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
>> 17 нояб. 2022 г., в 22:15, Adhemerval Zanella Netto <adhemerval.zanella@linaro.org> написал(а):
>>
>>
>>
>> On 17/11/22 05:47, Vladislav Khmelevsky wrote:
>>> This patch fixes two problems with audit:
>>> 1. The DL_OFFSET_RV_VPCS offset was mixed up with DL_OFFSET_RG_VPCS,
>>> resulting in x2 register value nulling in RG structure.
>>> 2. We need to preserve the x8 register before function call, but don't have
>>> to save it's new value and restore it before return. Anyway the final
>>> restore was using OFFSET_RV instead of OFFSET_RG value which is wrong (althoug doesn't affect anything).
>>
>> Patch looks ok, although I think currently the ABI only uses x0 and/or x1
>> to return value (for __int128_t for instance). So I think it should not
>> be a user-visible issue (at least tst-audit26 does check that lr_vpcs
>> and lrv_vpcs are zeroed). Are you seeing any issue with current code?
>> If so could you open a bug please?
>>
>>> ---
>>> sysdeps/aarch64/dl-trampoline.S | 4 +---
>>> 1 file changed, 1 insertion(+), 3 deletions(-)
>>>
>>> diff --git a/sysdeps/aarch64/dl-trampoline.S b/sysdeps/aarch64/dl-trampoline.S
>>> index 909b208578..d66f0b9c45 100644
>>> --- a/sysdeps/aarch64/dl-trampoline.S
>>> +++ b/sysdeps/aarch64/dl-trampoline.S
>>> @@ -298,12 +298,11 @@ _dl_runtime_profile:
>>> stp x2, x3, [x29, #OFFSET_RV + DL_OFFSET_RV_X0 + 16*1]
>>> stp x4, x5, [x29, #OFFSET_RV + DL_OFFSET_RV_X0 + 16*2]
>>> stp x6, x7, [x29, #OFFSET_RV + DL_OFFSET_RV_X0 + 16*3]
>>> - str x8, [x29, #OFFSET_RG + DL_OFFSET_RG_X0 + 16*4]
>>> stp q0, q1, [x29, #OFFSET_RV + DL_OFFSET_RV_V0 + 32*0]
>>> stp q2, q3, [x29, #OFFSET_RV + DL_OFFSET_RV_V0 + 32*1]
>>> stp q4, q5, [x29, #OFFSET_RV + DL_OFFSET_RV_V0 + 32*2]
>>> stp q6, q7, [x29, #OFFSET_RV + DL_OFFSET_RV_V0 + 32*3]
>>> - str xzr, [X29, #OFFSET_RV + DL_OFFSET_RG_VPCS]
>>> + str xzr, [X29, #OFFSET_RV + DL_OFFSET_RV_VPCS]
>>>
>>> /* Setup call to pltexit */
>>> ldp x0, x1, [x29, #OFFSET_SAVED_CALL_X0]
>>> @@ -315,7 +314,6 @@ _dl_runtime_profile:
>>> ldp x2, x3, [x29, #OFFSET_RV + DL_OFFSET_RV_X0 + 16*1]
>>> ldp x4, x5, [x29, #OFFSET_RV + DL_OFFSET_RV_X0 + 16*2]
>>> ldp x6, x7, [x29, #OFFSET_RV + DL_OFFSET_RV_X0 + 16*3]
>>> - ldr x8, [x29, #OFFSET_RV + DL_OFFSET_RV_X0 + 16*4]
>>> ldp q0, q1, [x29, #OFFSET_RV + DL_OFFSET_RV_V0 + 32*0]
>>> ldp q2, q3, [x29, #OFFSET_RV + DL_OFFSET_RV_V0 + 32*1]
>>> ldp q4, q5, [x29, #OFFSET_RV + DL_OFFSET_RV_V0 + 32*2]
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] elf: Fix rtld-audit trampoline for aarch64
2022-11-17 18:36 ` Adhemerval Zanella Netto via Libc-alpha
@ 2022-11-17 18:51 ` Vladislav Khmelevsky via Libc-alpha
2022-12-02 18:20 ` Carlos O'Donell via Libc-alpha
0 siblings, 1 reply; 6+ messages in thread
From: Vladislav Khmelevsky via Libc-alpha @ 2022-11-17 18:51 UTC (permalink / raw)
To: Adhemerval Zanella Netto; +Cc: libc-alpha, woodard
Thanks! But I would need some guidance from you if you don't mind. First of all I don't have write access to the repo, I would very appreciate if you or somebody would help me to merge this :)
> 17 нояб. 2022 г., в 22:36, Adhemerval Zanella Netto <adhemerval.zanella@linaro.org> написал(а):
>
>
>
> On 17/11/22 15:23, Vladislav Khmelevsky wrote:
>> Thank you for reviewing!
>> Yes, it is true tat inly x0/x1 are used as return register. But I have a specific audit library code that was storing some metadata in a free registers during plt entrer and reading it during plt exit :) As for a normal use cases both problems doesn't really affect anything.
>>
>
> Fair enough, although we might want to backport this.
>
> Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
>
>>> 17 нояб. 2022 г., в 22:15, Adhemerval Zanella Netto <adhemerval.zanella@linaro.org> написал(а):
>>>
>>>
>>>
>>> On 17/11/22 05:47, Vladislav Khmelevsky wrote:
>>>> This patch fixes two problems with audit:
>>>> 1. The DL_OFFSET_RV_VPCS offset was mixed up with DL_OFFSET_RG_VPCS,
>>>> resulting in x2 register value nulling in RG structure.
>>>> 2. We need to preserve the x8 register before function call, but don't have
>>>> to save it's new value and restore it before return. Anyway the final
>>>> restore was using OFFSET_RV instead of OFFSET_RG value which is wrong (althoug doesn't affect anything).
>>>
>>> Patch looks ok, although I think currently the ABI only uses x0 and/or x1
>>> to return value (for __int128_t for instance). So I think it should not
>>> be a user-visible issue (at least tst-audit26 does check that lr_vpcs
>>> and lrv_vpcs are zeroed). Are you seeing any issue with current code?
>>> If so could you open a bug please?
>>>
>>>> ---
>>>> sysdeps/aarch64/dl-trampoline.S | 4 +---
>>>> 1 file changed, 1 insertion(+), 3 deletions(-)
>>>>
>>>> diff --git a/sysdeps/aarch64/dl-trampoline.S b/sysdeps/aarch64/dl-trampoline.S
>>>> index 909b208578..d66f0b9c45 100644
>>>> --- a/sysdeps/aarch64/dl-trampoline.S
>>>> +++ b/sysdeps/aarch64/dl-trampoline.S
>>>> @@ -298,12 +298,11 @@ _dl_runtime_profile:
>>>> stp x2, x3, [x29, #OFFSET_RV + DL_OFFSET_RV_X0 + 16*1]
>>>> stp x4, x5, [x29, #OFFSET_RV + DL_OFFSET_RV_X0 + 16*2]
>>>> stp x6, x7, [x29, #OFFSET_RV + DL_OFFSET_RV_X0 + 16*3]
>>>> - str x8, [x29, #OFFSET_RG + DL_OFFSET_RG_X0 + 16*4]
>>>> stp q0, q1, [x29, #OFFSET_RV + DL_OFFSET_RV_V0 + 32*0]
>>>> stp q2, q3, [x29, #OFFSET_RV + DL_OFFSET_RV_V0 + 32*1]
>>>> stp q4, q5, [x29, #OFFSET_RV + DL_OFFSET_RV_V0 + 32*2]
>>>> stp q6, q7, [x29, #OFFSET_RV + DL_OFFSET_RV_V0 + 32*3]
>>>> - str xzr, [X29, #OFFSET_RV + DL_OFFSET_RG_VPCS]
>>>> + str xzr, [X29, #OFFSET_RV + DL_OFFSET_RV_VPCS]
>>>>
>>>> /* Setup call to pltexit */
>>>> ldp x0, x1, [x29, #OFFSET_SAVED_CALL_X0]
>>>> @@ -315,7 +314,6 @@ _dl_runtime_profile:
>>>> ldp x2, x3, [x29, #OFFSET_RV + DL_OFFSET_RV_X0 + 16*1]
>>>> ldp x4, x5, [x29, #OFFSET_RV + DL_OFFSET_RV_X0 + 16*2]
>>>> ldp x6, x7, [x29, #OFFSET_RV + DL_OFFSET_RV_X0 + 16*3]
>>>> - ldr x8, [x29, #OFFSET_RV + DL_OFFSET_RV_X0 + 16*4]
>>>> ldp q0, q1, [x29, #OFFSET_RV + DL_OFFSET_RV_V0 + 32*0]
>>>> ldp q2, q3, [x29, #OFFSET_RV + DL_OFFSET_RV_V0 + 32*1]
>>>> ldp q4, q5, [x29, #OFFSET_RV + DL_OFFSET_RV_V0 + 32*2]
>>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] elf: Fix rtld-audit trampoline for aarch64
2022-11-17 18:51 ` Vladislav Khmelevsky via Libc-alpha
@ 2022-12-02 18:20 ` Carlos O'Donell via Libc-alpha
0 siblings, 0 replies; 6+ messages in thread
From: Carlos O'Donell via Libc-alpha @ 2022-12-02 18:20 UTC (permalink / raw)
To: Vladislav Khmelevsky, Adhemerval Zanella Netto; +Cc: libc-alpha, woodard
On 11/17/22 13:51, Vladislav Khmelevsky via Libc-alpha wrote:
> Thanks! But I would need some guidance from you if you don't mind.
> First of all I don't have write access to the repo, I would very
> appreciate if you or somebody would help me to merge this :)
I was doing some review of rtld-audit changes we might want to backport, and
I just wanted to close the loop here that this is committed to the glibc
development branch as of November 21st and will be in glibc 2.37, and has
already been backported all the way to glibc 2.34 (which introduced the key
fixes for aarch64 in rtld-audit).
We could have written a test case for this, but it's an odd testcase
because it's about ensuring we touch minimal register state as we return
through the pltexit case.
Sending this email mostly for my own records so I remember what we did.
--
Cheers,
Carlos.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-12-02 18:20 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-17 8:47 [PATCH] elf: Fix rtld-audit trampoline for aarch64 Vladislav Khmelevsky via Libc-alpha
2022-11-17 18:15 ` Adhemerval Zanella Netto via Libc-alpha
2022-11-17 18:23 ` Vladislav Khmelevsky via Libc-alpha
2022-11-17 18:36 ` Adhemerval Zanella Netto via Libc-alpha
2022-11-17 18:51 ` Vladislav Khmelevsky via Libc-alpha
2022-12-02 18:20 ` Carlos O'Donell via Libc-alpha
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).