topjohnwu / Magisk

The Magic Mask for Android
GNU General Public License v3.0
47.59k stars 12.08k forks source link

Mounting magisk.img with nosuid breaks adbd on TouchWiz 7.0 #12

Closed chenxiaolong closed 7 years ago

chenxiaolong commented 7 years ago

On the new TouchWiz 7.0 beta for the S7 Flat/Edge, /sbin (bind mounted from /data/magisk.img) gets mounted with the nosuid option. This prevents /init from launching anything in /sbin (like adbd) because the kernel requires the SELinux policy to have a type_bounds statement when executing from a nosuid mountpoint: https://github.com/torvalds/linux/blob/v4.8/security/selinux/hooks.c#L2348.

This is not a problem for most ROMs since adbd is started before the Magisk hooks run, but TouchWiz 7.0 tries to restart adbd at the end of the boot process and it can no longer run.

I think the easiest way forward is to ensure that /data/magisk.img is mounted without nosuid, but I'm not sure what causes that option to be added in the first place.

EDIT: It looks like this is caused by CONFIG_RKP_NS_PROT=y in the stock kernel. Can we avoid bind-mounting /sbin to make su work?

chenxiaolong commented 7 years ago

Kmsg errors when nosuid /sbin is mounted and adbd is trying to start:

[ 1789.959062]  [0:           init:    1] init: Starting service 'adbd'...
[ 1789.962797]  [4:           init:14304] init: cannot execve('/sbin/adbd'): Permission denied
[ 1789.964128]  [0:           init:    1] init: Service 'adbd' (pid 14304) exited with status 127
[ 1789.964159]  [0:           init:    1] init: Service 'adbd' (pid 14304) killing any children in process group

Audit log when /init tries to start /sbin/adbd:

[V] type=1401 audit(1479073818.474:633): op=security_bounded_transition seresult=denied oldcontext=u:r:init:s0 newcontext=u:r:adbd:s0
[V] type=1300 audit(1479073818.474:633): arch=c00000b7 syscall=221 success=no exit=-13 a0=7f7e32ab61 a1=7f7e23d9e0 a2=750098 a3=1 items=0 ppid=1 pid=15068 auid=4294967295 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=(none) ses=4294967295 comm="init" exe="/init" subj=u:r:init:s0 key=(null)
[V] type=1327 audit(1479073818.474:633): proctitle=2F696E6974002D2D7365636F6E642D7374616765

Mountpoints:

hero2lte:/data # mount | grep loop0
/dev/block/loop0 on /magisk type ext4 (rw,seclabel,nosuid,noatime,data=ordered)
/dev/block/loop0 on /system/etc/hosts type ext4 (rw,seclabel,nosuid,noatime,data=ordered)
/dev/block/loop0 on /sbin type ext4 (rw,seclabel,nosuid,noatime,data=ordered)

Of course, this issue no longer happens if I umount -l /sbin to go back to the rootfs /sbin.

chenxiaolong commented 7 years ago

I've created https://github.com/Magisk-Modules-Repo/phh-superuser/pull/1 to fix the issue. While it works fine, I don't know if it has any repercussions for things like SafetyNet.

topjohnwu commented 7 years ago

I had to use bind mounts due to Safety Net, or else I will also just remount ramdisk to rw and add the binary lol. Please test if the new options makes any difference. Personally I will also restart adbd from time to time (wireless adb), that's why I released the r266-2 version that preserves the original binaries. BTW, manipulating PATH will also break Safety Net, that's why I had to mess with /sbin because bind mount su to /system/xbin is not an option (will break SN under any circumstances) Maybe another way is to mount a new directory on top of /sbin in magiskhide, but if setting the suid flag fixes the issue, then I'll just leave it as it now.

chenxiaolong commented 7 years ago

Ugh, I figured SafetyNet would be the reason for that.

if setting the suid flag fixes the issue, then I'll just leave it as it now.

Yep, this would be the ideal solution. The only problem is that the new TouchWiz kernels don't allow that.

topjohnwu commented 7 years ago

So what you mean is that even if I added the additional flag in the mount command (added in this commit 85dc669), TW kernel still don't accept mounting the image with suid?

chenxiaolong commented 7 years ago

Yes, that's correct. The kernel strips out MS_NOSUID no matter if you use the mount command or if you call mount("/dev/block/loopN", "/magisk", "ext4", MS_NOSUID, "") directory from C.

topjohnwu commented 7 years ago

@chenxiaolong, is it possible to run the binaries with suid within tmpfs? I thinking of mounting the /sbin with tmpfs, and copy the binaries over. Will that work?

chenxiaolong commented 7 years ago

@topjohnwu I just gave that a try and it seems that tmpfs is also forced mounted with MS_NOSUID by the kernel.

topjohnwu commented 7 years ago

@chenxiaolong I finally come up a way which might fix it. The attached zip is a beta build, what might fix the issue is moving /sbin -> /sbin_orig, then link the binaries to the bind mounted /sbin. Not sure if this will fix this issue, so I think I'd let you test before pushing to Github.

BTW, in newer versions, phh superuser is included into Magisk, you can just flash the zip, no need to flash a separate phh zip. Magisk-v10-beta.zip

chenxiaolong commented 7 years ago

@topjohnwu It works! The real binaries are on non-nosuid rootfs, so the fact that /sbin is nosuid doesn't matter anymore.

/sbin/adbd can now start successfully and su continues to work fine.

topjohnwu commented 7 years ago

Thanks, I'm glad I finally fixed this issue