[meta-ti][master/scarthgap][PATCH v1] meta-ti: Linux configs: Bring selinux related configs

[meta-ti][master/scarthgap][PATCH v1] meta-ti: Linux configs: Bring selinux related configs

[Aashvij Shenai]

Kernel configs that are important for SELinux to be included in the
Linux kernel are present in the meta-selinux layer.

Ideally, we wouldn't want to be calling a file from another
layer since it would become messy. However, bringing those configs
into meta-ti layer would hit maintainbility issues.

The root cause of this problem lies in the recipe name. While
meta-selinux names their bbapend as linux_yocto_%.bbappend, TI has their
recipe named as linux-ti-staging_%.bb

Signed-off-by: Aashvij Shenai <a-shenai@ti.com>
---
 .../linux/linux-ti-staging_%.bbappend             | 15 +++++++++++++++
 1 file changed, 15 insertions(+)
 create mode 100644 meta-ti-bsp/recipes-kernel/linux/linux-ti-staging_%.bbappend

diff --git a/meta-ti-bsp/recipes-kernel/linux/linux-ti-staging_%.bbappend b/meta-ti-bsp/recipes-kernel/linux/linux-ti-staging_%.bbappend
new file mode 100644
index 00000000..460df5de
--- /dev/null
+++ b/meta-ti-bsp/recipes-kernel/linux/linux-ti-staging_%.bbappend
@@ -0,0 +1,15 @@
+# The meta-selinux layer includes an selinux.cfg file containing
+# configs necessary for the Linux kernel to enable SELinux
+
+# In order to reduce maintainability issues, the file will 
+# be retained in meta-selinux layer
+
+FILESEXTRAPATHS:prepend := "${@bb.utils.contains('DISTRO_FEATURES', 'selinux', '${TOPDIR}/../sources/meta-selinux/recipes-kernel/linux/files:', '', d)}"
+
+SRC_URI += "${@bb.utils.contains('DISTRO_FEATURES', 'selinux', 'file://selinux.cfg', '', d)}"
+
+do_configure:append() {
+    if echo "${DISTRO_FEATURES}" | grep -q "selinux"; then
+        cat ${WORKDIR}/selinux.cfg >> ${B}/.config
+    fi
+}
\ No newline at end of file
-- 
2.34.1

Re: [meta-ti][master/scarthgap][PATCH v1] meta-ti: Linux configs: Bring selinux related configs

[Denys Dmytriyenko]

There are several issues with the patch itself, but that is secondary.

The primary issue is that this adds a new layer dependency to meta-ti-bsp 
(which is also not explicitly configured in layer.conf) - that is a very 
undesirable thing for a BSP layer.

This should be done in a Distro or Product layer (I'm not even shure 
meta-arago-distro should have this by default, to be honest).


On Thu, Nov 14, 2024 at 11:18:08AM +0530, Aashvij Shenai via lists.yoctoproject.org wrote:
> Kernel configs that are important for SELinux to be included in the
> Linux kernel are present in the meta-selinux layer.
> 
> Ideally, we wouldn't want to be calling a file from another
> layer since it would become messy. However, bringing those configs
> into meta-ti layer would hit maintainbility issues.
> 
> The root cause of this problem lies in the recipe name. While
> meta-selinux names their bbapend as linux_yocto_%.bbappend, TI has their
> recipe named as linux-ti-staging_%.bb
> 
> Signed-off-by: Aashvij Shenai <a-shenai@ti.com>
> ---
>  .../linux/linux-ti-staging_%.bbappend             | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
>  create mode 100644 meta-ti-bsp/recipes-kernel/linux/linux-ti-staging_%.bbappend
> 
> diff --git a/meta-ti-bsp/recipes-kernel/linux/linux-ti-staging_%.bbappend b/meta-ti-bsp/recipes-kernel/linux/linux-ti-staging_%.bbappend
> new file mode 100644
> index 00000000..460df5de
> --- /dev/null
> +++ b/meta-ti-bsp/recipes-kernel/linux/linux-ti-staging_%.bbappend
> @@ -0,0 +1,15 @@
> +# The meta-selinux layer includes an selinux.cfg file containing
> +# configs necessary for the Linux kernel to enable SELinux
> +
> +# In order to reduce maintainability issues, the file will 
> +# be retained in meta-selinux layer
> +
> +FILESEXTRAPATHS:prepend := "${@bb.utils.contains('DISTRO_FEATURES', 'selinux', '${TOPDIR}/../sources/meta-selinux/recipes-kernel/linux/files:', '', d)}"
> +
> +SRC_URI += "${@bb.utils.contains('DISTRO_FEATURES', 'selinux', 'file://selinux.cfg', '', d)}"
> +
> +do_configure:append() {
> +    if echo "${DISTRO_FEATURES}" | grep -q "selinux"; then
> +        cat ${WORKDIR}/selinux.cfg >> ${B}/.config
> +    fi
> +}
> \ No newline at end of file
> -- 
> 2.34.1
> 

Re: [meta-ti][master/scarthgap][PATCH v1] meta-ti: Linux configs: Bring selinux related configs

[Ryan Eatmon]

On 11/13/2024 11:48 PM, Aashvij Shenai wrote:
> Kernel configs that are important for SELinux to be included in the
> Linux kernel are present in the meta-selinux layer.
> 
> Ideally, we wouldn't want to be calling a file from another
> layer since it would become messy. However, bringing those configs
> into meta-ti layer would hit maintainbility issues.
> 
> The root cause of this problem lies in the recipe name. While
> meta-selinux names their bbapend as linux_yocto_%.bbappend, TI has their
> recipe named as linux-ti-staging_%.bb
> 
> Signed-off-by: Aashvij Shenai <a-shenai@ti.com>
> ---
>   .../linux/linux-ti-staging_%.bbappend             | 15 +++++++++++++++
>   1 file changed, 15 insertions(+)
>   create mode 100644 meta-ti-bsp/recipes-kernel/linux/linux-ti-staging_%.bbappend
> 
> diff --git a/meta-ti-bsp/recipes-kernel/linux/linux-ti-staging_%.bbappend b/meta-ti-bsp/recipes-kernel/linux/linux-ti-staging_%.bbappend
> new file mode 100644
> index 00000000..460df5de
> --- /dev/null
> +++ b/meta-ti-bsp/recipes-kernel/linux/linux-ti-staging_%.bbappend
> @@ -0,0 +1,15 @@
> +# The meta-selinux layer includes an selinux.cfg file containing
> +# configs necessary for the Linux kernel to enable SELinux
> +
> +# In order to reduce maintainability issues, the file will
> +# be retained in meta-selinux layer
> +
> +FILESEXTRAPATHS:prepend := "${@bb.utils.contains('DISTRO_FEATURES', 'selinux', '${TOPDIR}/../sources/meta-selinux/recipes-kernel/linux/files:', '', d)}"

NAK.  We cannot hard code paths like this.  This assumes several things 
about our layer-config and directory structure that is not present in 
all use cases of the meta-ti layer for all customers.  We cannot accept 
this patch as is.

I just tested the following:

require ${@bb.utils.contains('DISTRO_FEATURES', 'selinux', 
'recipes-kernel/linux/linux-yocto_selinux.inc', '', d)}

And it works fine.  It pulls in the file from meta-selinux without 
needing a bunch of other fluff.


> +SRC_URI += "${@bb.utils.contains('DISTRO_FEATURES', 'selinux', 'file://selinux.cfg', '', d)}"
> +
> +do_configure:append() {
> +    if echo "${DISTRO_FEATURES}" | grep -q "selinux"; then
> +        cat ${WORKDIR}/selinux.cfg >> ${B}/.config
> +    fi
> +}
> \ No newline at end of file

-- 
Ryan Eatmon                reatmon@ti.com
-----------------------------------------
Texas Instruments, Inc.  -  LCPD  -  MGTS

Re: [meta-ti][master/scarthgap][PATCH v1] meta-ti: Linux configs: Bring selinux related configs

[Ryan Eatmon]

On 11/14/2024 1:17 PM, Denys Dmytriyenko wrote:
> There are several issues with the patch itself, but that is secondary.
> 
> The primary issue is that this adds a new layer dependency to meta-ti-bsp
> (which is also not explicitly configured in layer.conf) - that is a very
> undesirable thing for a BSP layer.

That is a very good point.  I just sent my own reply to this patch 
listing the issues with it.  But this is a very very good point.

Question though... if the inclusion of the setup files in meta-selinux 
are hidden behind a DISTRO_FEATURES then are we really adding a 
dependency on meta-selinux?  Or are we adding a conditional dependency 
that assumes that if you are turning on "selinux" then meta-selinux must 
be in the bblayers.conf?




> This should be done in a Distro or Product layer (I'm not even shure
> meta-arago-distro should have this by default, to be honest).
> 
> 
> On Thu, Nov 14, 2024 at 11:18:08AM +0530, Aashvij Shenai via lists.yoctoproject.org wrote:
>> Kernel configs that are important for SELinux to be included in the
>> Linux kernel are present in the meta-selinux layer.
>>
>> Ideally, we wouldn't want to be calling a file from another
>> layer since it would become messy. However, bringing those configs
>> into meta-ti layer would hit maintainbility issues.
>>
>> The root cause of this problem lies in the recipe name. While
>> meta-selinux names their bbapend as linux_yocto_%.bbappend, TI has their
>> recipe named as linux-ti-staging_%.bb
>>
>> Signed-off-by: Aashvij Shenai <a-shenai@ti.com>
>> ---
>>   .../linux/linux-ti-staging_%.bbappend             | 15 +++++++++++++++
>>   1 file changed, 15 insertions(+)
>>   create mode 100644 meta-ti-bsp/recipes-kernel/linux/linux-ti-staging_%.bbappend
>>
>> diff --git a/meta-ti-bsp/recipes-kernel/linux/linux-ti-staging_%.bbappend b/meta-ti-bsp/recipes-kernel/linux/linux-ti-staging_%.bbappend
>> new file mode 100644
>> index 00000000..460df5de
>> --- /dev/null
>> +++ b/meta-ti-bsp/recipes-kernel/linux/linux-ti-staging_%.bbappend
>> @@ -0,0 +1,15 @@
>> +# The meta-selinux layer includes an selinux.cfg file containing
>> +# configs necessary for the Linux kernel to enable SELinux
>> +
>> +# In order to reduce maintainability issues, the file will
>> +# be retained in meta-selinux layer
>> +
>> +FILESEXTRAPATHS:prepend := "${@bb.utils.contains('DISTRO_FEATURES', 'selinux', '${TOPDIR}/../sources/meta-selinux/recipes-kernel/linux/files:', '', d)}"
>> +
>> +SRC_URI += "${@bb.utils.contains('DISTRO_FEATURES', 'selinux', 'file://selinux.cfg', '', d)}"
>> +
>> +do_configure:append() {
>> +    if echo "${DISTRO_FEATURES}" | grep -q "selinux"; then
>> +        cat ${WORKDIR}/selinux.cfg >> ${B}/.config
>> +    fi
>> +}
>> \ No newline at end of file
>> -- 
>> 2.34.1
>>

-- 
Ryan Eatmon                reatmon@ti.com
-----------------------------------------
Texas Instruments, Inc.  -  LCPD  -  MGTS

Re: [meta-ti][master/scarthgap][PATCH v1] meta-ti: Linux configs: Bring selinux related configs

[Aashvij Shenai]

Thanks for the reviews.

On 15/11/24 02:11, Ryan Eatmon wrote:
>
>
> On 11/14/2024 1:17 PM, Denys Dmytriyenko wrote:
>> There are several issues with the patch itself, but that is secondary.
>>
>> The primary issue is that this adds a new layer dependency to 
>> meta-ti-bsp
>> (which is also not explicitly configured in layer.conf) - that is a very
>> undesirable thing for a BSP layer.
>
> That is a very good point.  I just sent my own reply to this patch 
> listing the issues with it.  But this is a very very good point.
>
> Question though... if the inclusion of the setup files in meta-selinux 
> are hidden behind a DISTRO_FEATURES then are we really adding a 
> dependency on meta-selinux?  Or are we adding a conditional dependency 
> that assumes that if you are turning on "selinux" then meta-selinux 
> must be in the bblayers.conf?
>
Would moving an updated linux-ti-staging_%.bbappend under 
`meta-arago/meta-arago-distro/recipes-kernel`or 
`meta-arago/meta-arago-extras/recipes-kernel` resolve this impasse?

>
>
>
>> This should be done in a Distro or Product layer (I'm not even shure
>> meta-arago-distro should have this by default, to be honest).
>>
>>
>> On Thu, Nov 14, 2024 at 11:18:08AM +0530, Aashvij Shenai via 
>> lists.yoctoproject.org wrote:
>>> Kernel configs that are important for SELinux to be included in the
>>> Linux kernel are present in the meta-selinux layer.
>>>
>>> Ideally, we wouldn't want to be calling a file from another
>>> layer since it would become messy. However, bringing those configs
>>> into meta-ti layer would hit maintainbility issues.
>>>
>>> The root cause of this problem lies in the recipe name. While
>>> meta-selinux names their bbapend as linux_yocto_%.bbappend, TI has 
>>> their
>>> recipe named as linux-ti-staging_%.bb
>>>
>>> Signed-off-by: Aashvij Shenai <a-shenai@ti.com>
>>> ---
>>>   .../linux/linux-ti-staging_%.bbappend             | 15 
>>> +++++++++++++++
>>>   1 file changed, 15 insertions(+)
>>>   create mode 100644 
>>> meta-ti-bsp/recipes-kernel/linux/linux-ti-staging_%.bbappend
>>>
>>> diff --git 
>>> a/meta-ti-bsp/recipes-kernel/linux/linux-ti-staging_%.bbappend 
>>> b/meta-ti-bsp/recipes-kernel/linux/linux-ti-staging_%.bbappend
>>> new file mode 100644
>>> index 00000000..460df5de
>>> --- /dev/null
>>> +++ b/meta-ti-bsp/recipes-kernel/linux/linux-ti-staging_%.bbappend
>>> @@ -0,0 +1,15 @@
>>> +# The meta-selinux layer includes an selinux.cfg file containing
>>> +# configs necessary for the Linux kernel to enable SELinux
>>> +
>>> +# In order to reduce maintainability issues, the file will
>>> +# be retained in meta-selinux layer
>>> +
>>> +FILESEXTRAPATHS:prepend := "${@bb.utils.contains('DISTRO_FEATURES', 
>>> 'selinux', 
>>> '${TOPDIR}/../sources/meta-selinux/recipes-kernel/linux/files:', '', 
>>> d)}"
>>> +
>>> +SRC_URI += "${@bb.utils.contains('DISTRO_FEATURES', 'selinux', 
>>> 'file://selinux.cfg', '', d)}"
>>> +
>>> +do_configure:append() {
>>> +    if echo "${DISTRO_FEATURES}" | grep -q "selinux"; then
>>> +        cat ${WORKDIR}/selinux.cfg >> ${B}/.config
>>> +    fi
>>> +}
>>> \ No newline at end of file
>>> -- 
>>> 2.34.1
>>>
>
Regards,

Aashvij