[musl] Some pending changes/patches

[musl] Some pending changes/patches

[Érico Nogueira]

Hi!

Firstly, congratulations on the new release :)

Secondly, I have some pending patches that I'd like to ping, plus some
changes that I was asked to post about once the 1.2.2 release had been
made.

- add cloexec flag to open() in pthread_setname_np:
https://www.openwall.com/lists/musl/2020/12/24/1

- (low-priority) add pthread_getname_np:
	- my version (needs to be cleaned up for the change in the patch
	  above):
	  https://www.openwall.com/lists/musl/2020/12/19/3
	  It tries to avoid duplicating the setup for the
	  /proc/self/task/%d/com path by adding a helper function.
	- older version:
	  https://inbox.vuxu.org/musl/AM0PR07MB442007DDD4FECFF99BB5EBF48BE70@AM0PR07MB4420.eurprd07.prod.outlook.com/
	  Lacks cloexec flag when opening file.

- (low priority) implement gethostid() beyond a stub:
https://inbox.vuxu.org/musl/20200804224230.27774-1-ericonr@disroot.org/

- fix type for __libc_start_main. In crt1.c and rcrt1.c, it is:

int __libc_start_main(int (*)(), int, char **,
	void (*)(), void(*)(), void(*)());

but in __libc_start_main.c, it is

int __libc_start_main(int (*main)(int,char **,char **), int argc, char **argv)

as far as I can tell the fix is simple and the signature mismatch isn't
an issue. Since I don't know the context for the mismatch and therefore
wouldn't be able to write an appropriate commit message, I refrained
from sending a patch for it.

Happy new year,
Érico

Re: [musl] Some pending changes/patches

[Érico Nogueira]

Sorry for the noise, I forgot one patch:

- use SYS_openat instead of SYS_open in open(2) to avoid spurious
fcntl syscalls:
https://inbox.vuxu.org/musl/20201229225518.3677-1-ericonr@disroot.org/

All the questions in the patch still apply, so I don't believe it's 
ready to be merged, but a review would be very helpful - pointers on the 
matter of the kernel timeline regarding SYS_openat and O_CLOEXEC flag 
would also be appreciated. And maybe it should be using __syscall_cp for 
the syscall (I believe this makes a difference for cancellation points)?

Cheers

Em 17/01/2021 16:42, Érico Nogueira escreveu:
> Hi!
> 
> Firstly, congratulations on the new release :)
> 
> Secondly, I have some pending patches that I'd like to ping, plus some
> changes that I was asked to post about once the 1.2.2 release had been
> made.
> 
> - add cloexec flag to open() in pthread_setname_np:
> https://www.openwall.com/lists/musl/2020/12/24/1
> 
> - (low-priority) add pthread_getname_np:
> 	- my version (needs to be cleaned up for the change in the patch
> 	  above):
> 	  https://www.openwall.com/lists/musl/2020/12/19/3
> 	  It tries to avoid duplicating the setup for the
> 	  /proc/self/task/%d/com path by adding a helper function.
> 	- older version:
> 	  https://inbox.vuxu.org/musl/AM0PR07MB442007DDD4FECFF99BB5EBF48BE70@AM0PR07MB4420.eurprd07.prod.outlook.com/
> 	  Lacks cloexec flag when opening file.
> 
> - (low priority) implement gethostid() beyond a stub:
> https://inbox.vuxu.org/musl/20200804224230.27774-1-ericonr@disroot.org/
> 
> - fix type for __libc_start_main. In crt1.c and rcrt1.c, it is:
> 
> int __libc_start_main(int (*)(), int, char **,
> 	void (*)(), void(*)(), void(*)());
> 
> but in __libc_start_main.c, it is
> 
> int __libc_start_main(int (*main)(int,char **,char **), int argc, char **argv)
> 
> as far as I can tell the fix is simple and the signature mismatch isn't
> an issue. Since I don't know the context for the mismatch and therefore
> wouldn't be able to write an appropriate commit message, I refrained
> from sending a patch for it.
> 
> Happy new year,
> Érico
> 

Re: [musl] Some pending changes/patches

[Rich Felker]

On Sun, Jan 17, 2021 at 04:42:31PM -0300, Érico Nogueira wrote:
> - fix type for __libc_start_main. In crt1.c and rcrt1.c, it is:
> 
> int __libc_start_main(int (*)(), int, char **,
> 	void (*)(), void(*)(), void(*)());
> 
> but in __libc_start_main.c, it is
> 
> int __libc_start_main(int (*main)(int,char **,char **), int argc, char **argv)
> 
> as far as I can tell the fix is simple and the signature mismatch isn't
> an issue. Since I don't know the context for the mismatch and therefore
> wouldn't be able to write an appropriate commit message, I refrained
> from sending a patch for it.

I've wanted to fix this, but actually I don't know if we can; it needs
analysis. Defining the function with the unused junk args imposes an
ABI constraint that the caller (__libc_start_main) is entered with
argument space for 6 arguments. On some pass-by-register archs this is
not a constraint at all, but on pass-by-stack archs or archs where the
ABI requires the caller to reserve stack slots for the callee to spill
argument registers into, it does. If there's not sufficient space
reserved here, __libc_start_main could clobber space that overlaps
with argv[].

Now, crt1.c makes the call correctly with space reserved for 6
arguments. But prior to the switch to crt1.c and crt_arch.h, there was
separate per-arch asm making the call to __libc_start_main, and some
archs might have omitted the stack space for these slots. So this old
asm needs to be read to determine if there may be binaries calling the
function as if it were a 3-arg one.

I *think* we're okay here. The asm was removed in commit
6fef8cafbd0f6f185897bc87feb1ff66e2e204e1, and at that time (2015), all
the remaining asm versions seemed to still be passing the extra 3
args, despite __libc_start_main having dropped use of them much
earlier (2013, commit 7586360badcae6e73f04eb1b8189ce630281c4b2). But I
would like to review it further before making a change here.

Rich