[PATCH v7 09/23] aarch64: Try to free the GCS of makecontext

[Adhemerval Zanella Netto]

On 10/01/25 10:01, Yury Khrustalev wrote:
> On Thu, Jan 09, 2025 at 03:53:14PM -0300, Adhemerval Zanella Netto wrote:
>>
>>>> I think this will make makecontext non async-signal-safe when GCS is used,
>>>> when we explict document it as AS-Safe and AC-Safe.  I think using
>>>> internal_signal_block_all/internal_signal_restore_set would be suffice,
>>>> but it is also a performance regression.
>>>
>>> Do you refer to the call to malloc() in record_gcs()?
>>>
>>> We need to keep track of shadow stacks allocated via map_shadow_stack()
>>> when a new context is created with makecontext() so that we can munmap
>>> it when it is no longer required.
>>>
>>> I think this is not on a performance critical path, so we can add the
>>> internal_signal_block_all / internal_signal_restore_set pair. Could you
>>> recommend where it would be best to use it?
>>
>> Yes, glibc malloc is not async-signal-safe and thus calling on makecontext
>> make it async-signal-unsafe (similar for setcontext, which ends up calling
>> free). 
>>
>> I am not sure if you can really use malloc here, since a makecontext call 
>> potentially interrupt malloc itself. We had the same issue on the getrandom 
>> vDSO call; where we ended up using mmap directly instead. 
>>
>> You will still need to block/unblock signal to avoid reentrant signal
>> handlers; and there is the consideration of extra the mmap overhead per thread.
>>
>> Another possibility, which I think would be simpler, is to add a TCB buffer
>> of N entries and use instead of a linked-list.  It would add an small
>> memory overhead on each thread, and it would limit the number of in-flight 
>> makecontext a thread can make; but at least makecontext can to return 
>> ENOMEM if it can not create a new context, and it way simpler than adding
>> the mmap-allocator.
> 
> makecontext() does not return a value and doesn't set errno, so we probably
> cannot "return" ENOMEM. In existing code, when makecontext() failes, it calls
> abort(). Would the same be appropriate when we reach max number of context
> instances per thread?

Sigh, you are right (I confused it with setcontext for some reason).  I think
it would be the best option and x86_64 already does this if 
__allocate_shadow_stack fails.

> 
> Another option is to silently stop freeing shadow stacks after reaching max
> number. This will allow code that needs more makecontext's to continue with
> memory leak being downside (potentially leading to ENOMEM).

I don't think a silent leak is the best option, specially one that user can
not take some course of action without a lot of hackery.