Re: [PATCH 3/6] linux: Implement mremap in C

[Adhemerval Zanella Netto <adhemerval.zanella@linaro.org>]

On 06/06/24 16:38, enh wrote:
> On Thu, Jun 6, 2024 at 12:01 PM enh <enh@google.com> wrote:
>>
>> On Thu, Jun 6, 2024 at 11:15 AM Florian Weimer <fweimer@redhat.com> wrote:
>>>
>>> * Adhemerval Zanella via Libc-alpha:
>>>
>>>> Variadic function calls in syscalls.list does not work for all ABIs
>>>> (for instance where the argument are passed on the stack instead of
>>>> registers) and might have underlying issues depending of the variadic
>>>> type (for instance if a 64-bit argument is used).
>>>>
>>>> Checked on x86_64-linux-gnu.
>>>
>>> Where exactly does this break?

I don't really recall to be honest.  And I don't have a strong opinion, 
the main issue is I would like to eventually get rid of the assembly
hooks for the syscall auto-generation so we only have have it to support
for generate it through C.  Besides the code simplification it has the
side effect of playing better the LTO (which we still do not fully support,
but maybe in the future).

I had a prototype for this, which also is slight faster since it generates
the files in a bulk.

>>>
>>> If the syscall generator cannot handle pointer arguments, we should fix
>>> that.  As far as I can tell, we currently assume that the kernel can
>>> handle sign-extended pointers.  I don't see any special code for
>>> processing a/p/s pointers, for example.  That impacts far more than just
>>> mremap.  These pointers are generally not marked as U, for which we have
>>> special processing.
>>>
>>> Conditional varargs processing for system calls is very bad.  It causes
>>> an ongoing maintenance headache, and weird application issues.  For
>>> example, Linux 5.7 added MREMAP_DONTUNMAP, and it can use the address
>>> argument just like MREMAP_FIXED.  But the current mremap implementation
>>> always passes NULL as a system call argument.
>>
>> yeah, bionic and musl have this same bug, for the same reason of
>> having been written before MREMAP_DONTUNMAP was added, and not having
>> been updated since.
> 
> is it possible to use MREMAP_DONTUNMAP with a destination address
> without also setting MREMAP_FIXED? the kernel selftests don't seem to
> _test_ that case if so...
> 
> the glibc header comment also claims that MREMAP_FIXED is necessary.

The tools/testing/selftests/mm/mremap_dontunmap.c selftest does not have
an explicit test for MREMAP_DONTUNMAP with non-null variadic argument,
but it seems that all tests that require issue mremap with MREMAP_FIXED.

So, is this really an issue?