unofficial mirror of libc-alpha@sourceware.org
 help / color / mirror / Atom feed
From: Vladislav Khmelevsky via Libc-alpha <libc-alpha@sourceware.org>
To: Adhemerval Zanella Netto <adhemerval.zanella@linaro.org>
Cc: libc-alpha@sourceware.org, woodard@redhat.com
Subject: Re: [PATCH] elf: Fix rtld-audit trampoline for aarch64
Date: Thu, 17 Nov 2022 22:51:23 +0400	[thread overview]
Message-ID: <8E908272-FB79-4940-9183-A705EDA36D05@yandex.ru> (raw)
In-Reply-To: <3180cd8a-a3e2-b6bb-cb6a-af7c2a4fca22@linaro.org>

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]
>> 


  reply	other threads:[~2022-11-17 18:51 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
2022-12-02 18:20         ` Carlos O'Donell via Libc-alpha

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/libc/involved.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8E908272-FB79-4940-9183-A705EDA36D05@yandex.ru \
    --to=libc-alpha@sourceware.org \
    --cc=adhemerval.zanella@linaro.org \
    --cc=och95@yandex.ru \
    --cc=woodard@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).