whatawurst / android_device_sony_yoshino-common

This is the Android device configuration for the yoshino platform
10 stars 48 forks source link

This solves recent build issues with sepolicy. #99

Closed jw96 closed 1 year ago

jw96 commented 1 year ago

I don't know if it is secure to be honest.

derfelot commented 1 year ago

Hi,

what's the error this is meant to be fixing? Myself, I am just having an issue with lineage health sepolicy in android_device_lineage_sepolicy, which we can't really fix on our side:

neverallow check failed at out/soong/.intermediates/system/sepolicy/plat_pub_versioned.cil/android_common/plat_pub_versioned.cil:6276
  (neverallow domain sysfs_type (dir (write create link rename add_name remove_name reparent rmdir)))
    <root>
    allow at out/soong/.intermediates/system/sepolicy/vendor_sepolicy.cil/android_common/vendor_sepolicy.cil:8848
      (allow hal_lineage_health_default sysfs_battery_supply (dir (ioctl read write getattr lock open watch watch_reads add_name remove_name search)))
    <root>
    allow at out/soong/.intermediates/system/sepolicy/vendor_sepolicy.cil/android_common/vendor_sepolicy.cil:8851
      (allow hal_lineage_health_default sysfs_usb_supply (dir (ioctl read write getattr lock open watch watch_reads add_name remove_name search)))

neverallow check failed at out/soong/.intermediates/system/sepolicy/plat_sepolicy.cil/android_common/plat_sepolicy.cil:8723 from system/sepolicy/public/domain.te:1270
  (neverallow domain sysfs_type (dir (write create link rename add_name remove_name reparent rmdir)))
    <root>
    allow at out/soong/.intermediates/system/sepolicy/vendor_sepolicy.cil/android_common/vendor_sepolicy.cil:8848
      (allow hal_lineage_health_default sysfs_battery_supply (dir (ioctl read write getattr lock open watch watch_reads add_name remove_name search)))
    <root>
    allow at out/soong/.intermediates/system/sepolicy/vendor_sepolicy.cil/android_common/vendor_sepolicy.cil:8851
      (allow hal_lineage_health_default sysfs_usb_supply (dir (ioctl read write getattr lock open watch watch_reads add_name remove_name search)))

I'll see what can be done.

petefoth commented 1 year ago

what's the error this is meant to be fixing? Myself, I am just having an issue with lineage health sepolicy in android_device_lineage_sepolicy, which we can't really fix on our side:

In my build (of iode4.2) that error caused the following build failure

[  0% 273/64338] //system/sepolicy:sepolicy.recovery Compiling cil files for sepolicy.recovery [common]
FAILED: out/soong/.intermediates/system/sepolicy/sepolicy.recovery/android_common/sepolicy
out/host/linux-x86/bin/secilc -m -M true -G -c 30 out/soong/.intermediates/system/sepolicy/recovery_sepolicy.cil/android_common/recovery_sepolicy.cil -o out/soong/.intermediates/system/sepolicy/sepolicy.recovery/android_common/sepolicy_policy -f /dev/null && cp -f out/soong/.intermediates/system/sepolicy/sepolicy.recovery/android_common/sepolicy_policy out/soong/.intermediates/system/sepolicy/sepolicy.recovery/android_common/sepolicy && rm -f out/soong/.intermediates/system/sepolicy/sepolicy.recovery/android_common/sepolicy_policy # hash of input list: 187605db6ee3f7580bafd9adbd0101d2c2a0d02f423bb7efa74ee537c43d35ce
neverallow check failed at out/soong/.intermediates/system/sepolicy/recovery_sepolicy.cil/android_common/recovery_sepolicy.cil:11420 from system/sepolicy/public/domain.te:1271
  (neverallow domain sysfs_type (dir (write create link rename add_name remove_name reparent rmdir)))
    <root>
    allow at out/soong/.intermediates/system/sepolicy/recovery_sepolicy.cil/android_common/recovery_sepolicy.cil:41049
      (allow hal_lineage_health_default sysfs_battery_supply (dir (ioctl read write getattr lock open watch watch_reads add_name remove_name search)))
    <root>
    allow at out/soong/.intermediates/system/sepolicy/recovery_sepolicy.cil/android_common/recovery_sepolicy.cil:41052
      (allow hal_lineage_health_default sysfs_usb_supply (dir (ioctl read write getattr lock open watch watch_reads add_name remove_name search)))

Failed to generate binary
Failed to build policydb

The changes in this PR fixed that error and allowed the build to succeed

derfelot commented 1 year ago

yeah, so the real issue is here: https://github.com/LineageOS/android_device_lineage_sepolicy/blob/lineage-20.0/qcom/vendor/hal_lineage_health_default.te

This gives the lineage health hal dir write access to sysfs nodes, which turns out to be a neverallow (and shouldn't even be needed). So using the rw_dir_file macro here is a problem.

I will probably send a patch to LOS gerrit later today to address this. What is really strange though, is that no other devices are hitting this apparently.

derfelot commented 1 year ago

right, my bad. I totally missed the fact that we had our own macro for rw_dir_perms. Now LOS also has a public one, but it's defined differently (with only r_dir perms, whereas the one in our device tree has rw_dir_perms): https://github.com/LineageOS/android_device_lineage_sepolicy/commit/cb326751878ea021e03ea143330a7b40b8ed28a3

So we are probably overwriting the public one with the in-tree one, resulting in giving health hal rw dir perms on sysfs.

I will solve this in a different way, by just dropping our in-tree definition completely. It is literally only used once for thermal anyway.

derfelot commented 1 year ago

Fixed by https://github.com/whatawurst/android_device_sony_yoshino-common/pull/100