Skip to content

Memory allocation for ulp_stack using brk syscall #243

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 10, 2025

Conversation

dubeyabhishek
Copy link
Contributor

This patch addresses the mmap livepatching issue[1] by allocating ulp_stack through the brk() syscall.

Rather than using the brk() system call interface
provided by glibc, brk is invoked directly via glibc's syscall() function. This change shifts the existing limitation from mmap() to glibc's syscall().
However, glibc's brk() function remains available
for future patching.

Benefits of the New Approach:

1)The simplicity of the syscall() interface reduces
the vulnerability surface compared to mmap().
Hence, non-patchable exception for syscall is better.

2)It eliminates the need to copy the content of the
old ulp_stack after resizing.

[1] #239

@giulianobelinassi
Copy link
Collaborator

I am not sure this is a good idea. Mmap is signal safe and can safely be called within interruptions and sinal handling. Doesnt sbrk being called through syscall generate an interruption?

Furthermore, doesn't the call to sbrk disturb malloc in some way that may harm the application? When I used to code for embed systems, malloc could call sbrk to expand the heap towards the stack if necessary.

@susematz
Copy link
Collaborator

Yeah, I don't see any synchronization between the multiple thread break values, nor with the one used by the libc malloc. So it's easily possible with the patch for two threads to get the same current break and then try to set the same new break, making both threads ulp_stack[] be the same memory. Or the same situation just with the heap managed by malloc and now scribbled over by some threads ulp_stack[]. This can't work as-is.

@giulianobelinassi
Copy link
Collaborator

However, perhaps we should call mmap through a syscall routine, but instead of calling syscall directly, generate the interrupt to mmap through inline assembly. That would also make syscall livepatchable. Although syscall is very small and may not hide any bugs?

@dubeyabhishek
Copy link
Contributor Author

@giulianobelinassi Yes, going with mmap syscall is better choice. syscall is hardly few lines, so having it in patchable set is just the matter of completeness. But, if we go with inline assembly avoiding syscall, we need to handle return error codes. This will be redundant to tail of syscall interface.

@giulianobelinassi
Copy link
Collaborator

@giulianobelinassi Yes, going with mmap syscall is better choice. syscall is hardly few lines, so having it in patchable set is just the matter of completeness. But, if we go with inline assembly avoiding syscall, we need to handle return error codes. This will be redundant to tail of syscall interface.

Yeah. Just call syscall. I guess we can ignore that function.

This patch addresses the mmap livepatching issue[1]
by invoking mmap system call through glibc's syscall
interface. Hence, enabling livepatching on mmap
library function.

This change shifts the existing limitation from
mmap to syscall of glibc. Excluding syscall from
livepatchable set is acceptable due to its
minimal design.

[1] SUSE#239

Signed-off-by: Abhishek Dubey <[email protected]>
@dubeyabhishek
Copy link
Contributor Author

@giulianobelinassi done changes

@giulianobelinassi giulianobelinassi merged commit 4baef1b into SUSE:master Apr 10, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants