On Wed, May 09, 2018 at 09:23:05PM +0800, Yongqin Liu wrote: > Submitted a temporary workaround on selinux rules here: > https://android-review.googlesource.com/c/platform/system/sepolicy/+/682121 > > in case the re-order of events has any problem.
Thanks for doing this, I added all relevant people there to review. I still want to understand what changed a little better, so I'll get back when I go through the changes first. - ssp > > Thanks, > Yongqin Liu > > On 8 May 2018 at 14:47, Yongqin Liu <yongqin....@linaro.org> wrote: > > > > > > > On 8 May 2018 at 12:15, Sandeep Patil <sspa...@google.com> wrote: > > > >> > >> > >> On Mon, May 7, 2018 at 10:46 AM Stephen Smalley <s...@tycho.nsa.gov> > >> wrote: > >> > >>> On 05/07/2018 01:17 PM, Yongqin Liu wrote: > >>> > > >>> > > >>> > On 8 May 2018 at 00:55, Stephen Smalley <s...@tycho.nsa.gov <mailto: > >>> s...@tycho.nsa.gov>> wrote: > >>> > > >>> > On 05/07/2018 12:51 PM, Stephen Smalley wrote: > >>> > > On 05/07/2018 12:30 PM, Yongqin Liu wrote: > >>> > >> I run the commands as root with userdebug build, after run > >>> su command. > >>> > > > >>> > > Can you run id -Z before and after running su? I'm trying to > >>> understand why the scontext is u:r:kernel:s0 instead of e.g. u:r:shell:s0 > >>> (regular shell) or u:r:su:s0 (su shell). > >>> > > >>> > h01:04:28 liuyq: ~$ adb shell > >>> > hikey:/ $ id > >>> > uid=2000(shell) gid=2000(shell) groups=2000(shell),1004(input) > >>> ,1007(log),1011(adb),1015(sdcard_rw),1028(sdcard_r),3001( > >>> net_bt_admin),3002(net_bt),3003(inet),3006(net_bw_stats),3009(readproc),3011(uhid) > >>> context=u:r:shell:s0 > >>> > hikey:/ $ su > >>> > hikey:/ # id > >>> > uid=0(root) gid=0(root) groups=0(root),1004(input),100 > >>> 7(log),1011(adb),1015(sdcard_rw),1028(sdcard_r),3001(net_ > >>> bt_admin),3002(net_bt),3003(inet),3006(net_bw_stats),3009(readproc),3011(uhid) > >>> context=u:r:su:s0 > >>> > hikey:/ # ^D > >>> > hikey:/ $ ^D > >>> > 01:05:52 liuyq: ~$ adb shell > >>> > hikey:/ $ > >>> > hikey:/ $ id -Z > >>> > context=u:r:shell:s0 > >>> > hikey:/ $ su > >>> > hikey:/ # id -Z > >>> > context=u:r:su:s0 > >>> > hikey:/ # > >>> > > >>> > > >>> > > >>> > Is it because it is a console rather than adb and there is no > >>> domain transition defined for shell execution from the console? Should > >>> there be a domain_auto_trans(kernel, shell_exec, shell) rule in policy? > >>> > > >>> > Both running it from the serial console after su, or the via vts with > >>> adb shell(after adb root), will report kernel scontext domain. > >>> > > >>> > > >>> > Actually, we don't allow kernel domain to execute anything other > >>> than init, so I don't understand how you got a shell running in kernel > >>> domain (if that is in fact what you did). > >>> > > >>> > > >>> > it's what i want to be clear too. > >>> > >>> Ok, the implication is that the actual write request is happening from a > >>> kernel thread, not in the context of the process that is performing the > >>> mkfs command, e.g. the write is deferred to a work queue or similar > >>> mechanism. If so, then there isn't much point in performing the check at > >>> all, because it will always be from the kernel domain regardless of the > >>> userspace originator. > >>> > >>> I'm not sure that moving the security_file_permission() calls into > >>> rw_verify_area() was a good idea since a userspace permissions check is > >>> logically different than the other kinds of validation being performed > >>> there. However, I think it > >>> was motivated by the fact that originally all callers of > >>> rw_verify_area() were also performing a security_file_permission() call, > >>> and to help ensure that no future read/write interfaces bypassed the > >>> security check. > >>> > >>> The underlying hook function, selinux_file_permission(), only performs a > >>> permission check if something has changed since the checks performed at > >>> open time, e.g. the current process' sid differs from that of the opener, > >>> the inode SID has changed, or the policy has changed. In this case, I > >>> assume it is because the writer is running in the kernel domain whereas > >>> the > >>> opener was in the domain of the process that invoked mkfs, e.g. su. > >>> > >>> The near term fix is to simply allow it for the kernel domain under > >>> userdebug_or_eng(). > >> > >> > >> ... or simply re-order the events as ... > >> > >> 1. create a file > >> 2. format it with mkfs.extN > >> 3. .. then assign a loopback device > >> 4. mount (this used to only generate read access from kernel domain) > >> > >> That is what LTP tests have been doing and seems like this particular > >> one swaps #2 & #3 which causes the 'write' check. We didn't want the > >> kernel > >> domain to have write access anywhere IIRC, so the policy change is > >> correct. > >> > >> Once the filesystem is created and mounted, all reads/writes come > >> from userspace domain anyway. > >> > >> Unless, the problem is with step #4 above and that has now changed with > >> 4.14. I'll check on my end to see if this is the case .. > >> > >> The failed LTP testcases are following: > > > > VtsKernelLtp#fs.fs_fill_64bit > > VtsKernelLtp#syscalls.fallocate04_64bit > > VtsKernelLtp#syscalls.fallocate05_64bit > > VtsKernelLtp#syscalls.fchmod06_64bit > > VtsKernelLtp#syscalls.fsync01_64bit > > VtsKernelLtp#syscalls.ftruncate04_64_64bit > > VtsKernelLtp#syscalls.ftruncate04_64bit > > VtsKernelLtp#syscalls.link08_64bit > > VtsKernelLtp#syscalls.linkat02_64bit > > VtsKernelLtp#syscalls.mkdirat02_64bit > > VtsKernelLtp#syscalls.mknod07_64bit > > VtsKernelLtp#syscalls.mknodat02_64bit > > VtsKernelLtp#syscalls.msync04_64bit > > VtsKernelLtp#syscalls.rmdir02_64bit > > VtsKernelLtp#syscalls.setxattr01_64bit > > VtsKernelLtp#syscalls.mount01_64bit > > VtsKernelLtp#syscalls.mount02_64bit > > VtsKernelLtp#syscalls.mount06_64bit > > VtsKernelLtp#syscalls.umount01_64bit > > VtsKernelLtp#syscalls.umount02_64bit > > VtsKernelLtp#syscalls.umount03_64bit > > VtsKernelLtp#syscalls.umount2_01_64bit > > VtsKernelLtp#syscalls.umount2_02_64bit > > VtsKernelLtp#syscalls.umount2_03_64bit > > VtsKernelLtp#syscalls.utime06_64bit > > VtsKernelLtp#syscalls.utimes01_64bit > > > > > > -- > > Best Regards, > > Yongqin Liu > > --------------------------------------------------------------- > > #mailing list > > linaro-andr...@lists.linaro.org <linaro-...@lists.linaro.org> > > http://lists.linaro.org/mailman/listinfo/linaro-android > > > > > > -- > Best Regards, > Yongqin Liu > --------------------------------------------------------------- > #mailing list > linaro-andr...@lists.linaro.org <linaro-...@lists.linaro.org> > http://lists.linaro.org/mailman/listinfo/linaro-android
