On 09/09/2015 04:17 AM, weiyuan wrote:
> Thanks to your kindly help.
>
> On 2015/9/9 0:44, Stephen Smalley wrote:
>> On 09/08/2015 05:34 AM, weiyuan wrote:
>>> Dear All:
>>>
>>> On Android 6.0,
>>>
>>> I have a file "/sys/class/leds/red/brightness" under /sys, its parent
>>> directory is a symlink.
>>>
>>> "u:object_r:sysfs:s0 red ->
>>> ../../devices/fff34000.pmic/pmic_led.118/leds/red"
>>> "u:object_r:sysfs:s0 brightness"
>>>
>>> I notice that there is a patch "restorecon: only operate on canonical
>>> paths.",
>>> so I add some logs like "--SELINUX--:" in the function
>>> "selinux_android_restorecon_common", then I runs some tests.
>>>
>>> -----------test A.-----------
>>>
>>> file_contexts:
>>> "/sys/class/leds/red/brightness u:object_r:sysfs_led:s0"
>>>
>>> # restorecon /sys/class/leds/red/brightness
>>> => "--SELINUX--: selabel_lookup failed. pathname =
>>> /sys/devices/fff34000.pmic/pmic_led.118/leds/red/brightness"
>>>
>>> # ls -Z /sys/class/leds/red/brightness
>>> u:object_r:sysfs:s0 brightness unchanged
>>>
>>>
>>> restorecon find the realpath of "brightness" has no match in
>>> file_contexts, so it failed.
>>>
>>> -----------test B.-----------
>>>
>>> file_contexts:
>>> "/sys/class/leds/red(/.*)? u:object_r:sysfs_led:s0"
>>>
>>> # restorecon -Rv /sys/class/leds/red
>>> => "--SELinux--: pathname
>>> =/sys/devices/fff34000.pmic/pmic_led.118/leds/red"
>>>
>>> # ls -Z /sys/class/leds/red
>>> u:object_r:sysfs:s0 red ->
>>> ../../devices/fff34000.pmic/pmic_led.118/leds/red unchanged
>>>
>>>
>>> restorecon find the realpath of "red" has no match in file_contexts, so it
>>> failed.
>>> -----------test C.-----------
>>>
>>> file_contexts:
>>> "/sys/class/leds/red(/.*)? u:object_r:sysfs_led:s0"
>>>
>>> # restorecon -Rv /sys/class/leds
>>> => "--SELINUX--:selabel_lookup failed. pathname = /sys/class/leds
>>> SELinux: Relabeling /sys/class/leds/red from u:object_r:sysfs:s0 to
>>> u:object_r:sysfs_led:s0."
>>>
>>> # ls -Z /sys/class/leds
>>> u:object_r:sysfs_led:s0 red ->
>>> ../../devices/fff34000.pmic/pmic_led.118/leds/red changed
>>>
>>> # cd /sys/class/leds/red
>>> # ls -Z
>>> u:object_r:sysfs:s0 brightness
>>> unchanged
>>>
>>> # ls -Z /sys/devices/fff34000.pmic/pmic_led.118/leds
>>> u:object_r:sysfs:s0 red unchanged
>>>
>>>
>>> restorecon find the realpath of "leds" has a match in file_contexts, so
>>> set "red" successed;
>>> BUT it failed to set files in "red". And the original file's selable is
>>> unchanged.
>>>
>>> -----------test D.-----------
>>> Use "stat" on these files:
>>> "/sys/class/leds/red" and "/sysdevices/fff34000.pmic/pmic_led.118/leds/red"
>>> are different inodes。
>>> "/sys/class/leds/red/brightness" and
>>> "/sysdevices/fff34000.pmic/pmic_led.118/leds/red/brightness" are the same
>>> inode.
>>> (Which means that any change on
>>> realpath"/sysdevices/fff34000.pmic/pmic_led.118/leds/red/brightness" will
>>> simultaneously reflect on the symlink file
>>> "/sys/class/leds/red/brightness" )
>>
>> Note: /sys/class/leds/red/brightness is NOT a symlink file. It is a
>> regular file that you looked up via the /sys/class/leds symlink. But
>> only /sys/class/leds/red is a symlink here.
>>
>>>
>>>
>>> My problem is :
>>> 1. The realpath of "/sys/class/leds/red" is various on different devices,
>>> but the symlink path is fixed.
>>> If I want to set the selabel of "/sys/class/leds/red/brightness", I
>>> have to
>>> add "[realpath]/brightness [label]" in file_contexts on every devices
>>> differently,
>>> because the realpath of "brightness" is different.
>>> Can this be done with other ways that not so inconvenient?
>>
>> How did you do it in Android 5.0? I guess there you could add a
>> "restorecon /sys/class/leds/red/brightness" command to your
>> init.<board>.rc file and it wouldn't try to canonicalize the path and
>> would therefore lookup the provided path and label it with the matching
>> context. Is that what you were doing previously?
>>
>
>
> Yes, that is exactly what I did in Android 5.0.
>
>
>>>
>>> 2. Can symlink and realpath have different selables?
>>
>> Yes, they resolve to different inodes (a symbolic link is its own
>> file/inode in Linux, separate and independent of the target). The
>> symbolic link SELinux label only controls access to the symlink (i.e.
>> the ability to unlink, rename, or read it), not access to its target
>> (which is controlled based on the target's label).
>>
>>> If they have different selables, what about the files in symlink
>>> directory, like "brightness"?
>>> Which selable should it follow, since it has only one inode exist.
>>
>> brightness is a regular file; only /sys/class/leds/red is a symlink, so
>> there is only one inode and one SELinux label to consider for brightness.
>>
>>> 3. If symlink and realpath can have different selables,
>>> I think the patch "restorecon: only operate on canonical paths." is not
>>> appropriate.
>>> If I want to set symlink's selable, I have to run restorecon on its
>>> parent directory,
>>> and it will only change the directory self, not the files in the
>>> directory.
>>> In the meanwhile, if I restorecon the symlink directory directly, it
>>> will fail.
>>> Is this a Bug?
>>
>> Yes and no; on the one hand, the patch broke something that used to work
>> in Android 5.0 IIUC; on the other hand, that approach only worked as a
>> hack. file_contexts is supposed to specify real paths (there is an
>> exception for /dev/block/platform/.../by-name symlinks, but that's
>> handled specially by init/ueventd). See
>> http://marc.info/?l=seandroid-list&m=142482974726583&w=2 and
>> https://android-review.googlesource.com/#/c/97701/
>>
>>> 4. How about enforce symlink and realpath have the same selable?
>>> When restorecon meet a symlink,
>>> 1) find the realpath
>>> 2) call selabel_lookup with the realpath, if failed, call
>>> selabel_lookup with the symlink.
>>> 3) use the selabel find in step 2) to set label to both symlink and
>>> realpath.
>>
>> This is somewhat akin to the selabel_lookup_best_match() concept used by
>> init/ueventd for the /dev symlinks, but is problematic for restorecon.
>> ueventd handles it at device file creation time, and knows the real path
>> and all of the symlink paths up front. restorecon is just walking the
>> file tree and relabeling files as it goes. When it reaches the real
>> file, it doesn't know about any symbolic links to it. When it reaches
>> one symbolic link, it doesn't know about any others. It might visit the
>> symbolic link, label the file to match your
>> /sys/class/leds/red/brightness entry, and then visit the real file later
>> and relabel it back to a different type. It may also be unsafe if used
>> on any directory tree that can be modified by an untrusted entity, as in
>> restorecon -R /data, since that could allow someone to force a sensitive
>> file to be relabeled by creating a symlink to it.
>>
>> However, your approach might work in the restricted case of sysfs, just
>> by virtue of the fact that there is no default /sys(/.*)? entry in
>> file_contexts (intentionally, so that we can bail immediately when there
>> is no specific match), so we are unlikely to have two conflicting
>> entries for the real path and the symlink path, and the /sys tree should
>> be safe against such manipulation.
>>
>> Nonetheless, it probably carries a cost - invoking realpath() on every
>> symlink under /sys that is visited could be expensive, although we are
>> at least pruning the tree walk based on selabel_partial_match(). Would
>> require some investigation and performance testing at least.
>>
>> Alternative would be to just revert the "restorecon: only operate on
>> canonical paths" change so you can still use restorecon commands in
>> init.<board>.rc files and have them "work" on paths containing symbolic
>> links. Not sure though what may be depending on that change.
>>
>>
>>
>>
>
> Yes, I'm afraid that revert the patch will cause some unknown issues.
>
> I notice that, when fts_read meets a symlink directory, it appears
> to not continue to scaning files in it but just stops there.
>
> How about add a -S flag to restorecon, which will act like:
> 1. If -R is presented, -S will be ignored. (Because for -R, I don't know
> how to make fts_read keep looking files under symlink directory)
> 2. With -S flag, restorecon will not invoke realpath, but directly
> go to restorecon_sb.
>
> This will change restorecon's behavior in two ways, one is that restorecon can
> set symlink's lable without change symlink's realpath's lable; another one is
> that restorecon can change a regular file's lable with any path the caller
> may provide.
>
> So it will keep thing that restorecon used to work in Android 5.0 somehow,and
> without reverting the patch.
init restorecon command is a built-in; it does not exec the toolbox
restorecon command, so adding options there won't help you with a
restorecon from init.<board>.rc.
The latter is implemented by system/core/init/builtins.cpp, in the
do_restorecon() function, which calls utils.cpp:restorecon(), which
calls selinux_android_restorecon() in libselinux with 0 flags argument.
init commands aren't supposed to take options; that's why it has
separate restorecon and restorecon_recursive built-in commands even
though underneath it is just passing different flags.
You could add a restorecon_norealpath or similar built-in command, and
have it pass a new NOREALPATH flag to selinux_android_restorecon(), and
use that to decide whether to call realpath or not. It isn't really
about whether or not the file is a symlink but whether you want realpath
used.
It isn't a general solution however, and it requires you to manually add
such restorecon_norealpath calls to your init.<board>.rc file. It would
be better to fix the underlying libselinux logic to handle it correctly
so that the existing restorecon_recursive("/sys") by init would just
label it the way you want in the first place.
_______________________________________________
Seandroid-list mailing list
[email protected]
To unsubscribe, send email to [email protected].
To get help, send an email containing "help" to
[email protected].
