Re: [RFC PATCH] selinux: Merge Android libselinux changes with upstream

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 04/27/2015 08:11 AM, Richard Haines wrote:
> This RFC patch takes the libselinux changes from AOSP master
> and merges them into the SELinux project source from
> https://github.com/SELinuxProject/selinux
> 
> checkpolicy, libselinux and libsepol have been updated with the
> appropriate Android.mk files and licences.
> 
> The majority of changes were minor and use ifdef/ifndef, however:
> 1) label_file.c from Android was renamed label_android_file.c
>    as the changes between them were great (Android does not
>    currently use the *.bin file_contexts file)
>    There is a request to add this support to Android (see
>    https://bitbucket.org/seandroid/wiki/wiki/ToDo), but left it
>    until later (just in case this patch does not fly at all).
> 
> 2) procattr.c has been modified to support both but looks messy.
> 
> 3) Not all Android changes were added as they did not seem necessary,
>    for example include/selinux/selinux.h was not modified.
> 
> 4) libselinux/src/Makefile was modified to build the required modules.
> 
> 5) The Android.mk file was modified to add src/enabled.c because
>    callbacks.c calls is_selinux_enabled.
>    Also for non-user TARGET_BUILD_VARIANT add booleans.c because
>    external/ltrace/sysdeps/linux-gnu/trace.c requires
>    security_get_boolean_active, otherwise booleans could be removed.
> 
> These changes have been tested on Fedora 21 and Android master
> builds (eng and user) using the emulator with no problems (the
> external/checkpolicy, external/libselinux and external/libsepol
> projects were removed).
> The testing was not extensive and consisted of checking bootloading,
> labeling and running apps (Android) and checking the libselinux/sepol
> functions on Fedora.
> 
> Signed-off-by: Richard Haines <richard_c_haines@xxxxxxxxxxxxxx>

NAK.  I agree with the end goal but not the approach taken here.

What I would prefer is a more incremental approach, e.g. (in no specific
order):

- port individual changes from upstream libselinux to Android libselinux
that occurred after the fork, such as the file_contexts.bin support (in
doing so, taking a careful look at that change and consider improving
it, as it looked a bit suspect to me; consider an ill-formed/corrupted
file_contexts.bin file and how well the current parsing code will handle
that situation),

- port individual changes from Android libselinux to upstream
libselinux, such as the extensions to the selabel interface and
label_file backend for best match and partial match support, and
consider using them for equivalent functionality in Linux distributions
for enabling labeling of device nodes based on stable partition names
and fast labeling of all of /sys at boot,

- wrap any residual glibc-specific dependencies in upstream libselinux
with conditional code to allow use of other libc implementations like
bionic,

- reconsider the handling of context translation in upstream libselinux;
it adds overhead to every non-raw call and mcstrans is not
installed/active by default, so this is wasted on most installs; for
Android, we made the _raw behavior the default and only behavior,

- reconsider the procattr cache in upstream libselinux; it was allegedly
added to improve performance but seems to have grown ever more complex
and has caused subtle breakages; it would be useful to revisit whether
it is in fact worth retaining,

- consider dropping some legacy functions/interfaces and compatibility
code from upstream libselinux that we intentionally dropped from the
Android libselinux,

- generalize all policy pathname definition/code in upstream libselinux
so that it works cleanly on Android or Linux distributions,

- Consider taking selinux_android_restorecon() to upstream libselinux as
a general selinux_restorecon() function and rewriting
policycoreutils/setfiles to use it instead.

Also, it makes no sense to add MODULE_LICENSE_* or NOTICE files to
upstream selinux because they merely duplicate information already in
upstream COPYING or LICENSE files just for the sake of conventions
preferred by Android.  Those additional files only need to exist in the
Android tree.  Possibly even the Android.mk files don't belong in the
upstream selinux tree since they are only ever used in the Android
build, but I did previously upstream the files for libsepol and
checkpolicy.  I don't think those are usually upstreamed however for
other external projects.

I would minimize use of #ifdefs, and only use them when it is truly an
Android-specific change.

I recognize that this approach is a lot more work and will take longer,
but I think it will yield a better result for both upstream and Android.

_______________________________________________
Selinux mailing list
Selinux@xxxxxxxxxxxxx
To unsubscribe, send email to Selinux-leave@xxxxxxxxxxxxx.
To get help, send an email containing "help" to Selinux-request@xxxxxxxxxxxxx.




[Index of Archives]     [Selinux Refpolicy]     [Linux SGX]     [Fedora Users]     [Fedora Desktop]     [Yosemite Photos]     [Yosemite Camping]     [Yosemite Campsites]     [KDE Users]     [Gnome Users]

  Powered by Linux