* Jeff Xu jeffxu@chromium.org [240910 10:15]:
Hi Muhammad
On Mon, Aug 19, 2024 at 3:05 AM Muhammad Usama Anjum Usama.Anjum@collabora.com wrote:
On 8/14/24 3:29 AM, Jeff Xu wrote:
Hi Muhammad
On Fri, Aug 9, 2024 at 1:25 AM Muhammad Usama Anjum usama.anjum@collabora.com wrote:
The __NR_mmap isn't found on armhf. The mmap() is commonly available system call and its wrapper is presnet on all architectures. So it should be used directly. It solves problem for armhf and doesn't create problem for architectures as well. Remove sys_mmap() functions as they aren't doing anything else other than calling mmap(). There is no need to set errno = 0 manually as glibc always resets it.
The mseal_test should't have dependency on libc, and mmap() is implemented by glibc, right ?
I just fixed a bug to switch mremap() to sys_mremap to address an issue that different glibc version's behavior is slightly different for mremap().
What is the reason that __NR_mmap not available in armhf ? (maybe it is another name ?) there must be a way to call syscall directly on armhf, can we use that instead ?
It seems __NR_mmap syscall is deprecated for arm. Found this comment in arch/arm/include/asm/unistd.h: /*
- The following syscalls are obsolete and no longer available for EABI:
- __NR_time
- __NR_umount
- __NR_stime
- __NR_alarm
- __NR_utime
- __NR_getrlimit
- __NR_select
- __NR_readdir
- __NR_mmap
- __NR_socketcall
- __NR_syscall
- __NR_ipc
*/
The glibc mmap() calls mmap2() these days by adjusting the parameters internally. From man mmap: C library/kernel differences: This page describes the interface provided by the glibc mmap() wrapper function. Originally, this function invoked a system call of the same name. Since Linux 2.4, that system call has been superseded by mmap2(2), and nowadays the glibc mmap() wrapper function invokes mmap2(2) with a suitably adjusted value for offset.
I'm not sure if behaviour of glibc mmap() and syscall mmap2() would be same, but we should use glibc at most places which accounts for different architectures correctly. Maybe the differences were only present in case of mremap().
We shouldn't use glibc to test mseal, mseal is a security feature, and an attacker can access syscall directly, so the test needs to test with as little layer as possible.
This sounds like you are concerned about the use of a library artificially reducing the attack surface that exists by bypassing the library to access the syscall directly.
If you have an example of something that is restricted by the library that can be used by the attacker, then we will need to roll our own caller with wrappers so that this works on random archs.
It appears that the existing tests can use the library without reducing the test coverage, so why would we maintain our own abstraction?
This patch is also upstream, so it is obviously not going to change at this point. I'm sure you are aware of that as you raised this concern on the failed backport email [1]. Neither of these locations are the right one for a discussion that you are trying to start. It would be better to produce an RFC patch and send it to the mm mailing list and cc the people on this patch.
Thanks, Liam
[1]. https://lore.kernel.org/all/CABi2SkV-FdDQy2bjDkpgpqz7hX7ybeTjCrUgUf6WcYqGkux...