termux / proot

An chroot-like implementation using ptrace.
https://wiki.termux.com/wiki/PRoot
Other
774 stars 160 forks source link

`/dev/ashmem` and `fstat`. #258

Open twaik opened 1 year ago

twaik commented 1 year ago

Problem description

Can you please update proot to update struct stat::st_size if fstat answer == 0, struct stat::st_size == 0 and file descriptor points to /dev/ashmem with return value of ashmem_get_size_region?

#define __ASHMEMIOC 0x77
#define ASHMEM_SET_SIZE _IOW(__ASHMEMIOC, 3, size_t)

int ashmem_get_size_region(int fd)
{
  return ioctl(fd, ASHMEM_GET_SIZE, NULL);
}

Steps to reproduce You can see steps and a code snippets in related issue. https://github.com/termux/termux-packages/issues/15239 .

Expected behavior struct stat::st_size should contain ashmem region size after fstat call.

Additional information Related issue.

michalbednarski commented 1 year ago

I think you can use following as alternative to shm_open/ashmem:

    int32_t shmfd = open(".", O_TMPFILE | O_RDWR | O_EXCL | O_CLOEXEC, 0); // Needs writeable directory
    int32_t shmfd = memfd_create("x", MFD_CLOEXEC);

(Or creating file and unlink()-ing it after opening, although I'm not sure if that doesn't actually not touch disk)


I'll later see how proposed behaviour can be implemented (although I'm pretty sure that is nontrivial and will require chaining ioctl after fstatat()/fstatat64() (depending on used libc), as proot itself cannot directly determine size of ashmem residing in another process)

twaik commented 1 year ago

open ... memfd_create...

open used this this will sync changes with disk and only after that I will be able to get data in other process. It is unacceptable because it will lead to a strong consumption of flash memory resources. memfd_create is disabled on android kernels (but available on some devices).

And there is a problem. I can not use it directly, it is already a part of Xorg source code and I can not override this behaviour. Users launch it in chroot and I should somehow handle this behavior.

Can you please also implement memfd_create with following ftruncate?

michalbednarski commented 1 year ago

Pushed experimental extension that redirects memfd_create (if not natively supported), ftruncate and fstat to ashmem

Extension needs to be enabled through --ashmem-memfd switch

twaik commented 1 year ago

Thank you. I am already working on code which will require this.

twaik commented 1 year ago

About shm_open. Does proot handle opening files in /dev/shm, /run/shm, /var/tmp and /tmp? Applications are actively using them for heavy operations like transfering large blobs between to other apps and opening ashmem regions instead of opening files in these locations can speed up those applications.

michalbednarski commented 1 year ago

Termux proot includes hacks for allowing shm_open (without which shm_open would fail altogether) and requires caller to provide writeable directory for /dev/shm through --bind

Replacing open there in my opinion risks more complexity and compatibility problems than it probably worth it (Unless you have real app where difference can be shown to be significant)

Opening ashmem/memfd by name would require receiving fd from another process, for example through SCM_RIGHTS (which is currently implemented in proot for shmat, although that procedure requires chaining multiple syscalls and was source of bugs before, also shmat emulation creates separate namespaces for multiple invocations of proot)

JanuszChmiel commented 1 year ago

I Am recommending to The Proot-distro to allow users to use this new implemented feature as soon as it will be statet as stable and secure. Mr Twaik thank you for your professional debate with MR Bednarski and thank you for your deep interest about C language and Proot. Very well done.

twaik commented 1 year ago

It looks like I was wrong and you can simply use memfd for shared memory fragments. It is better because it supports fstat without hacks.

Termux proot includes hacks for allowing shm_open (without which shm_open would fail altogether) and requires caller to provide writeable directory for /dev/shm through --bind

Replacing open there in my opinion risks more complexity and compatibility problems than it probably worth it (Unless you have real app where difference can be shown to be significant)

Opening ashmem/memfd by name would require receiving fd from another process, for example through SCM_RIGHTS (which is currently implemented in proot for shmat, although that procedure requires chaining multiple syscalls and was source of bugs before, also shmat emulation creates separate namespaces for multiple invocations of proot)

What about using pidfd_getfd to get fd of shared fragment? But anyway you will need something like termux's shm mechanism to have shared namespace between multiple proot processes...