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.





_______________________________________________
Seandroid-list mailing list
[email protected]
To unsubscribe, send email to [email protected].
To get help, send an email containing "help" to 
[email protected].

Reply via email to