Skip to content
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

/dev/ashmem and fstat. #258

Open
twaik opened this issue Feb 20, 2023 · 8 comments
Open

/dev/ashmem and fstat. #258

twaik opened this issue Feb 20, 2023 · 8 comments

Comments

@twaik
Copy link
Member

twaik commented Feb 20, 2023

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. termux/termux-packages#15239 .

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

Additional information
Related issue.

@michalbednarski
Copy link
Collaborator

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
Copy link
Member Author

twaik commented Feb 20, 2023

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 added a commit to termux/termux-packages that referenced this issue Feb 23, 2023
@michalbednarski
Copy link
Collaborator

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
Copy link
Member Author

twaik commented Feb 23, 2023

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

@twaik
Copy link
Member Author

twaik commented Mar 3, 2023

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
Copy link
Collaborator

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
Copy link

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
Copy link
Member Author

twaik commented Mar 23, 2023

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...

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

No branches or pull requests

3 participants