Created attachment 9771 [details] patch to fix the bug Hi all, I found the dynamic linker did not check the return value when allocating tlsblocks(csu/libc-tls.c and elf/rtld.c). There are some explanations in comments, but they failed to convince me. Users will only get a segment-fault when there are no memory for tlsblocks. They will get confused as there are no helping message printed. Moreover, there many similar checks for memory allocations. So ,why not do the same for tlsblocks? I have added a patch to fix the problem(see attachment), it that ok for trunk?
If the allocation fails, sbrk will crash because errno has not been set up at this point. There is really no good way to deal with this situation.
(In reply to Florian Weimer from comment #1) > If the allocation fails, sbrk will crash because errno has not been set up > at this point. There is really no good way to deal with this situation. Thanks for the reply. I did not notice that seterrno would lead to crash(segv), my bad... As to this problem， I think we still have many ways to solve it. IMO, the easiest plan is to use brk syscall directly instead of using __sbrk.
(In reply to ma.jiang from comment #2) > (In reply to Florian Weimer from comment #1) > > If the allocation fails, sbrk will crash because errno has not been set up > > at this point. There is really no good way to deal with this situation. > Thanks for the reply. I did not notice that seterrno would lead to > crash(segv), my bad... As to this problem， I think we still have many ways > to solve it. IMO, the easiest plan is to use brk syscall directly instead of > using __sbrk. We currently cannot use INTERNAL_SYSCALL in generic code, so this is more complicated than it looks.
(In reply to Florian Weimer from comment #3) > We currently cannot use INTERNAL_SYSCALL in generic code, so this is more > complicated than it looks. Hi Florian, Sorry for the delay, we got spring festival last week. If we could not use INTERNAL_SYSCALL in libc-tls.c then I can finally understand those comments in code now... But I still can not understand why INTERNAL_SYSCALL can not be used here. INTERNAL_SYSCALL is just a macro in sysdep.h, and have been used in many generic routines (nptl/nptl-init.c,sysdeps/unix/sysv/linux/clock_nanosleep.c,...). Moreover, I have tried to add a static __sbrk (which used INTERNAL_SYSCALL) into libc-tls.c. It seems all things are ok, no compiling errors or run-time errors. By the way, there is another piece of code in my patch. It is about init_tls in elf/rtld.c. This piece should be applicable , I suppose.
(In reply to ma.jiang from comment #4) > (In reply to Florian Weimer from comment #3) > > We currently cannot use INTERNAL_SYSCALL in generic code, so this is more > > complicated than it looks. > Hi Florian, > Sorry for the delay, we got spring festival last week. > If we could not use INTERNAL_SYSCALL in libc-tls.c then I can finally > understand those comments in code now... But I still can not understand why > INTERNAL_SYSCALL can not be used here. > INTERNAL_SYSCALL is just a macro in sysdep.h, and have been used in many > generic routines > (nptl/nptl-init.c,sysdeps/unix/sysv/linux/clock_nanosleep.c,...). With “generic“, I meant “not Linux-specific”. INTERNAL_SYSCALL is specific to Linux. Hurd uses something else. I don't know what the out-of-tree FreeBSD port uses.
Created attachment 9793 [details] patchV2 (In reply to Florian Weimer from comment #5) > (In reply to ma.jiang from comment #4) > With “generic“, I meant “not Linux-specific”. INTERNAL_SYSCALL is specific > to Linux. Hurd uses something else. I don't know what the out-of-tree > FreeBSD port uses. Thanks for your patient explanation. Now I fully understand the whole story. I have updated my patch(see the attachment), which trap SIGSEGV and print error strings. Could you take some time to review it?
Created attachment 9794 [details] patchv2-assume-sbrk-may-return-when-out-of-memory If sbrk could return when out of memory, this patch should be used.
This was fixed in glibc 2.36 (and backported): commit f787e138aa0bf677bf74fa2a08595c446292f3d7 Author: Florian Weimer <fweimer@redhat.com> Date: Mon May 16 18:41:43 2022 +0200 csu: Implement and use _dl_early_allocate during static startup This implements mmap fallback for a brk failure during TLS allocation. scripts/tls-elf-edit.py is updated to support the new patching method. The script no longer requires that in the input object is of ET_DYN type. Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>