Subject: bug#75322: SAFE_ALLOCA assumed to root Lisp_Objects/SSDATA(string)

From: Pip Cet <[email-masked]>
Date: Sat, 04 Jan 2025 11:29:46 +0000

Gerd Möllmann <[email-masked]> writes:

> Pip Cet <[email-masked]> writes:
>
>> Gerd Möllmann <[email-masked]> writes:
>>
>>> Gerd Möllmann <[email-masked]> writes:
>>>
>>>>
>>>> The pointers to string data case probably requires adding yet another
>>>> macro SAFE_ALLOCA_FIND_A_GOOD_NAME, which, for MPS, allocates a root,
>>>> possibly and exact one which would be good.
>>
>> Note that might still EFAULT if there's a memory barrier.  I think.
>>
>> Do we really need to move all arguments to syscalls and libc functions
>> which might use a syscall into non-MPS memory?  That would be bad.
>>
>> And which libc functions might use a syscall?  I think we can agree
>> fprintf might, and memcpy() doesn't (note to self: destroy all evidence
>> I ever considered making memcpy() use MMU tricks for very large
>> buffers), but what about all the others?
>>
>> Maybe I'm panicking too much and fixing read/write/exec* is good
>> enough?
>
> Don't Panic! Quote from The Hitchhiker's Guide to Emacs (non-NS edition)
> :-).

> TBH, I couldn't follow your thoughts above with the EFAULT, syscalls and
> so on.

My understanding is that if there is a memory barrier in place for a
string that a syscall tries to access, we get an -EFAULT from Linux, an
EFAULT from glibc, and the syscall won't work.

This is what makes valid_pointer_p work, for example.  (To the extent it
does: valid_pointer_p assumes 16 bytes after the pointer are readable; I
don't see why that is true for small objects).

What makes this more difficult is that glibc and GCC disagree about what
to do with invalid pointers even in the simplest case: glibc documents
printf ("%s\n", NULL) to work, but GCC will rewrite it into puts (NULL),
which crashes.  I'm worried that glibc might wrap a syscall incorrectly
wrt EFAULT and SIGSEGV, in this case.

Worse, if the syscall is in a fork()ed process, MPS machinery to remove
the memory barrier might not be in place after the fork.  And who knows
about posix_spawn action descriptors?  Or vfork?

>>> Or one does it as you did in b0a209e9204, that's of course also safe.
>>> For both old and new GC. (Don't remember if you mentioned it Pip, but
>>> old GC moves string data as well, during string compaction, should GC
>>> run).
>>
>> Ouch.  Yes, I remember now.
>>
>> Pip
>
> And today I see you reverted that commit. Is there something wrong with
> it? I couldn't see something wrong, and for me VALUE(no root) >
> VALUE(exact) VALUE(ambig).

There were two reasons for the revert:

1. Eli asked me not to push the change right after I pushed.  I thought
it would be best to restore the "before" state so we could discuss the
solution.

2. For the non-MPS case, I rashly assumed it would be okay to remove the
no-GC assumption that call_process apparently establishes (even though
there is no comment saying so).  I'm not sure what I would do now; the
old code seems buggy to me because Fexpand_file_name can call Lisp, but
that bug affects only argv, not envp.  It may be best to fix the argv
code but leave the envp code in its (once again) current fragile state,
documenting precisely which assumptions are made there.

> WRT Lisp_Object allocas, please tell if I should do that.

Sorry, I don't understand.  Lisp_Objects shouldn't be allocated with
SAFE_ALLOCA, but allocating them with SAFE_ALLOCA_LISP_EXTRA is fine.
Pointers to string data cannot currently be safely allocated with
SAFE_ALLOCA, but I'm not sure whether SAFE_ALLOCA_AMBIGUOUS or
SAFE_ALLOCA_EXACT_POINTER would be the right thing to do.

Pip