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

Refactor magic mount #7685

Merged
merged 5 commits into from
Mar 20, 2024
Merged

Refactor magic mount #7685

merged 5 commits into from
Mar 20, 2024

Conversation

yujincheng08
Copy link
Collaborator

Mirror was previously used for accessing the original file during magic mount when we are using a tmpfs to cover the target. However, since we introduce atomic mount, we switch all tmpfs mount in worker and then move to the target at once. It means that we can still access the original file when we are constructing the tmpfs mount point. Thus we no longer need mirror.

@yujincheng08 yujincheng08 force-pushed the nomirror branch 2 times, most recently from ebbe342 to 45bcbaa Compare January 4, 2024 16:50
@yujincheng08 yujincheng08 reopened this Jan 4, 2024
@yujincheng08 yujincheng08 linked an issue Jan 4, 2024 that may be closed by this pull request
@yujincheng08 yujincheng08 marked this pull request as draft January 4, 2024 18:22
@yujincheng08 yujincheng08 force-pushed the nomirror branch 3 times, most recently from ca5f892 to 6bebf40 Compare January 5, 2024 03:47
@vvb2060 vvb2060 marked this pull request as ready for review January 5, 2024 04:28
@reiryuki
Copy link
Contributor

reiryuki commented Jan 5, 2024

My magisk modules are detecting some original files from mirror while installing whether the rom is already have it or not.

if we cannot detect the original files, then the second installations after reboot will produce wrong detections which result in my modules will think the rom is already have the file (even the file is from the module at the first installation, not from the original rom), the the module will not use the file again. In result, the second installation will lose the file.

@vvb2060
Copy link
Collaborator

vvb2060 commented Jan 5, 2024

Magisk/docs/details.md

Lines 17 to 18 in a0b8aa4

# Magisk internal stuffs
INTERNALDIR=$MAGISKTMP/.magisk

.magisk is an internal directory, modules should not depend on it. I think it's better for the module to fix its incorrect behavior than to prevent magisk from developing

@vvb2060
Copy link
Collaborator

vvb2060 commented Jan 5, 2024

For your case, you can use magisk --denylist exec ls. Or fix the install logic, such as checking whether the file exists in both the system and module dir.

@reiryuki
Copy link
Contributor

reiryuki commented Jan 5, 2024

but magisk --path command will still available right?

and magisk --path/mirror dir

@yujincheng08
Copy link
Collaborator Author

yujincheng08 commented Jan 5, 2024

yes. but note that magisk --path does not point to the internal directory.

@reiryuki

This comment was marked as off-topic.

@vvb2060
Copy link
Collaborator

vvb2060 commented Jan 5, 2024

This command is deprecated but retained for compatibility reasons. You can replace it with the following script, which behave the same but support KSU.

MAGISKTMP=/sbin
[ -d /sbin ] || MAGISKTMP=/debug_ramdisk

Repository owner locked as off-topic and limited conversation to collaborators Jan 5, 2024
@canyie canyie linked an issue Jan 7, 2024 that may be closed by this pull request
@vvb2060 vvb2060 force-pushed the nomirror branch 2 times, most recently from 40b336a to c5f2b43 Compare January 12, 2024 01:51
@vvb2060
Copy link
Collaborator

vvb2060 commented Feb 22, 2024

I think it can fix #6914

yujincheng08 and others added 5 commits March 1, 2024 22:53
Mirror was previously used for accessing the original file during
magic mount when we are using a tmpfs to cover the target. However,
since we introduce atomic mount, we switch all tmpfs mount in
worker and then move to the target at once. It means that we can
still access the original file when we are constructing the tmpfs
mount point. Thus we no longer need mirror.
@topjohnwu topjohnwu merged commit ecc74d4 into topjohnwu:master Mar 20, 2024
15 checks passed
@yujincheng08 yujincheng08 deleted the nomirror branch March 20, 2024 06:22
@osm0sis
Copy link
Collaborator

osm0sis commented Mar 22, 2024

I just wanted to make one final comment on the mirror removal matter...

This is kinda too bad, since the mirror was an easy way to access and change the real partitions using root on devices that allow that, for both stock and custom ROMs.

That's quite a loss of function for the affected older Android versions, or any ROM not using ext4-dedup/erofs and could still allow rw system. Yes, it was an "internal" feature, but it is a loss for root nonetheless.

Repository owner unlocked this conversation Mar 22, 2024
@vvb2060
Copy link
Collaborator

vvb2060 commented Mar 22, 2024

Just unmount all magic mounts in a new private mount namespace. I believe a developer using magisk's internal details should be able to do it. Because this is easier than using mirror and is a documented, supported method.

@osm0sis
Copy link
Collaborator

osm0sis commented Mar 22, 2024

Just unmount all magic mounts in a new private mount namespace. I believe a developer using magisk's internal details should be able to do it. Because this is easier than using mirror and is a documented, supported method.

Hmm what about in a module post-fs-data.sh or other script?

@vvb2060
Copy link
Collaborator

vvb2060 commented Mar 22, 2024

magisk --denylist exec sh

@osm0sis
Copy link
Collaborator

osm0sis commented Mar 22, 2024

Oh wow, that's a very cool feature. Wasn't aware of it! Thanks! 🙌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
next Planned for the release *after* the next release
Projects
None yet
5 participants