[Bug]: Buffer overflow in __is_mmaped (glibc patches), and other problems

[Kamillaova]

Problem description

I'm sleepy now, so I don't want to write any explanations here, sorry. But I think you'll understand without explanations, thanks. UPD okay I essentially wrote it for other problems.

https://github.com/termux-pacman/glibc-packages/blob/63d5639e392fe3c6cdc16391ccc8845f32e1ef13/gpkg/glibc/mprotect.c#L53

Also:

• glibc supports compiling with -Werror=no-unused-variable, but it was broken here: https://github.com/termux-pacman/glibc-packages/blob/63d5639e392fe3c6cdc16391ccc8845f32e1ef13/gpkg/glibc/setegid.c.patch#L13
• glibc supports compiling with -Werror=maybe-uninitialized, but it was broken here (since r is not probably not initialized, and I don't quite understand what value r should have): https://github.com/termux-pacman/glibc-packages/blob/63d5639e392fe3c6cdc16391ccc8845f32e1ef13/gpkg/glibc/clock_gettime.c.patch#L7
• https://github.com/termux-pacman/glibc-packages/issues/292#issuecomment-2381080824

And... for the future: please create all patches with the -p flag, for better navigation through patches. -p, --show-c-function show which C function each change is in

[Kamillaova]

https://github.com/termux-pacman/glibc-packages/blob/63d5639e392fe3c6cdc16391ccc8845f32e1ef13/gpkg/glibc/mprotect.c#L93

hmm, I think mmap with MAP_FIXED to freeed pointer is intentional, but I think you should write a comment near this code that explaining this action. And also add a #pragma GCC diagnostic ignored .... for -Wuse-after-free, thanks.

[Kamillaova]

[Maxython]

I mostly have questions about the environment where you are trying to build glibc, especially looking at your repository. Here are the main ones:

• Are you trying to port the glibc package for termux to a NixOS package? If yes, do you use all sources and algorithms from glibc package for Termux?
• For compilation do you use cgct (special binutils and gcc) or regular gcc? If regular, can you tell me the version of your gcc?
• Can you show how you setted up flags (CFLAGS and LDFLAGS) to compile glibc?

Note that I have not worked with NixOS packages. I am an ArchLinux/Termux user. So I may have additional questions.

I'll also note that I'm working on a global glibc update (you can see it in the glibc branch). There are changes there that solve the problem with setegid.c.patch and clock_gettime.c.patch, which you noted. As for the mprotect.c code, I'd like to understand your compilation environment first. It's just that you already suggest making some changes (related to CFLAGS) to fix something in the glibc package that can successfully build without these changes in the configured environment of this repository (glibc-packages).

And... for the future: please create all patches with the -p flag, for better navigation through patches.

Okay, thanks for the advice.

[Kamillaova]

do you use all sources and algorithms from glibc package for Termux?

yes, all patches from //gpkg/glibc/build.sh termux_step_pre_configure are applied.

but configure flags are not identical, but I think that at this point it doesn't matter. configure flags: --disable-static --prefix=.../glibc-2.39-52 --bindir=.../glibc-2.39-52-bin/bin --sbindir=.../glibc-2.39-52-bin/sbin --includedir=.../glibc-2.39-52-dev/include --oldincludedir=.../glibc-2.39-52-dev/include --mandir=.../glibc-2.39-52-bin/share/man --infodir=.../glibc-2.39-52-bin/share/info --docdir=.../glibc-2.39-52/share/doc/glibc --libdir=.../glibc-2.39-52/lib --libexecdir=.../glibc-2.39-52/libexec --localedir=.../glibc-2.39-52/share/locale -C --enable-add-ons --sysconfdir=/etc --enable-stack-protector=strong --enable-bind-now --with-headers=.../linux-headers-6.9/include --disable-profile --enable-fortify-source --enable-static-pie --enable-kernel=3.10.0

If regular, can you tell me the version of your gcc?

regular gcc, gcc version 13.3.0 (GCC)

Can you show how you setted up flags (CFLAGS and LDFLAGS) to compile glibc? I can, but the main point of this issue is the main question, and second and third entry of Also:, and https://github.com/termux-pacman/glibc-packages/issues/292#issuecomment-2381114031. This is UB, and it is not good to leave it as is. (how does this even work lmao?)

buff[GLOBAL_READ_SIZE] = '\0'; is 99.9% written by accident, because this it is a buffer overflow. but mmap on free'ed pointer is also UB, but I think it was done intentionally... need explanation.

memset (with length of sizeof(char*) !!!) after malloc, what is this lol? I think you want to use calloc(strlc+strlen(buff), sizeof(char)) here?

the other problems are just real warnings, but glibc compiles fine with theese -Werror's, so if you're patching glibc, maybe it makes more sense to follow the glibc "code style"?

so mprotect.c is f*cked by UB and strange code by >90%, this is awful.

[Kamillaova]

Also I think some patches are aimed at solving proot issues, but AFAIK regular debian etc. runs on proot without problems, and I don't need proot patches because I don't want to use proot.

source: https://github.com/termux-pacman/glibc-packages/blob/63d5639e392fe3c6cdc16391ccc8845f32e1ef13/gpkg/glibc/mprotect.c#L5

Can you explain this? Is this only needed when using proot? Are there other patches that are only needed for proot?

[Kamillaova]

CFLAGS: gcc ../sysdeps/unix/sysv/linux/mprotect.c -c -std=gnu11 -fgnu89-inline -g -O2 -Wall -Wwrite-strings -Wundef -Werror -fmerge-all-constants -frounding-math -fstack-protector-strong -fno-common -Wp,-U_FORTIFY_SOURCE,-D_FORTIFY_SOURCE=3 -Wstrict-prototypes -Wold-style-definition -fmath-errno -fPIC -ftls-model=initial-exec -I../include -I/build/build/misc -I/build/build -I../sysdeps/unix/sysv/linux/aarch64 -I../sysdeps/aarch64/nptl -I../sysdeps/unix/sysv/linux/wordsize-64 -I../sysdeps/unix/sysv/linux/include -I../sysdeps/unix/sysv/linux -I../sysdeps/nptl -I../sysdeps/pthread -I../sysdeps/gnu -I../sysdeps/unix/inet -I../sysdeps/unix/sysv -I../sysdeps/unix -I../sysdeps/posix -I../sysdeps/aarch64/fpu -I../sysdeps/aarch64/multiarch -I../sysdeps/aarch64 -I../sysdeps/wordsize-64 -I../sysdeps/ieee754/ldbl-128 -I../sysdeps/ieee754/dbl-64 -I../sysdeps/ieee754/flt-32 -I../sysdeps/ieee754 -I../sysdeps/generic -I.. -I../libio -I. -nostdinc -isystem /data/data/com.termux/files/nix/store/km3ax94pqh5y15jz0i5x19dcdqxvdr26-xgcc-13.3.0/lib/gcc/aarch64-unknown-linux-gnu/13.3.0/include -isystem /data/data/com.termux/files/nix/store/km3ax94pqh5y15jz0i5x19dcdqxvdr26-xgcc-13.3.0/lib/gcc/aarch64-unknown-linux-gnu/13.3.0/include-fixed -isystem /data/data/com.termux/files/nix/store/xriwpw9d4c2s1qaq5nafvjbj959zy2rp-linux-headers-6.9/include -D_LIBC_REENTRANT -include /build/build/libc-modules.h -DMODULE_NAME=libc -include ../include/libc-symbols.h -DPIC -DSHARED -DTOP_NAMESPACE=glibc -o /build/build/misc/mprotect.os -MD -MP -MF /build/build/misc/mprotect.os.dt -MT /build/build/misc/mprotect.os

that may be not full list because of cc-wrapper's flags, but I think flags like FORTIFY_SOURCE etc. are not relevant here

[Maxython]

Ok, thanks for answering my questions. According to your CFLAGS you use some flags that enable error return. This will allow me to repeat similar compilation of glibc package.

Regarding the mprotect.c code, I tested your suggestions and fixed the error (that you were getting when compiling it). I will most likely merge these mprotect.c code changes into the glibc branch as part of a global update.

but mmap on free is also UB, but I think it was done intentionally... need explanation.

If I remember correctly, mprotect.c uses free() to free up memory space for the mmaped address.

Also I think some patches are aimed at solving proot issues, but AFAIK regular debian etc. runs on proot without problems, and I don't need proot patches because I don't want to use proot. Can you explain this? Is this only needed when using proot? Are there other patches that are only needed for proot?

This link, which is listed as "cause", was added as an analog explanation of the origin of the problem of running mprotect() with PROT_EXEC. That is, it was added not to point to the patch of this commit, but to point to the comment of this commit.

so mprotect.c is f*cked by UB and strange code by >90%, this is awful.

Thank you


But seriously, I understand. The code mprotect.c was added as a first attempt to bypass the selinux limitation, back in the early stages of glibc development. In the current glibc, it is a dirty hack (which gave an unforgettable experience of fixing the bug).

[Kamillaova]

If I remember correctly, mprotect.c uses free() to free up memory space for the mmaped address.

mmap on free'ed pointer*

[Maxython]

Try to build glibc with this mprotect.c.

PS: if the compilation succeeds, do not close this issue. It should be closed after merging the glibc branch with the main branch.