diff mbox series

ipe: Search for the boot policy file in the source tree

Message ID 20250122065740.545042-1-code@tyhicks.com (mailing list archive)
State Handled Elsewhere
Headers show
Series ipe: Search for the boot policy file in the source tree | expand

Commit Message

Tyler Hicks Jan. 22, 2025, 6:57 a.m. UTC 
Resolve CONFIG_IPE_BOOT_POLICY relative file paths in the source tree if
the file was not found within the object tree and is not an absolute path.

This fixes an IPE build failure that occurs when using an output directory,
such as with the `O=/tmp/build` make option, during a build with the
CONFIG_IPE_BOOT_POLICY option set to a path that's relative to the kernel
source tree. For example,

  $ grep CONFIG_IPE_BOOT_POLICY /tmp/build/.config
  CONFIG_IPE_BOOT_POLICY="ipe-boot-policy"
  $ touch ipe-boot-policy
  $ make O=/tmp/build
  make[1]: Entering directory '/tmp/build'
    GEN     Makefile
    UPD     include/config/kernel.release
    UPD     include/generated/utsrelease.h
    CALL    scripts/checksyscalls.sh
    CC      init/version.o
    AR      init/built-in.a
    CC      kernel/sys.o
    AR      kernel/built-in.a
    IPE_POL ipe-boot-policy
  An error occurred during policy conversion: : No such file or directory
  make[5]: *** [security/ipe/Makefile:14: security/ipe/boot_policy.c] Error 2
  make[4]: *** [scripts/Makefile.build:440: security/ipe] Error 2
  make[3]: *** [scripts/Makefile.build:440: security] Error 2
  make[2]: *** [Makefile:1989: .] Error 2
  make[1]: *** [Makefile:251: __sub-make] Error 2
  make[1]: Leaving directory '/tmp/build'
  make: *** [Makefile:251: __sub-make] Error 2

Fixes: ba199dc909a2 ("scripts: add boot policy generation program")
Cc: stable@vger.kernel.org
Signed-off-by: Tyler Hicks <code@tyhicks.com>
---
 security/ipe/Makefile | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Fan Wu Jan. 22, 2025, 6:41 p.m. UTC | #1 
Thanks for the fix.

My only concern here is the use of wildcard. I'm not sure but if
$(CONFIG_IPE_BOOT_POLICY) is a glob pattern it could match multiple
files?

Other than that I think the doc of security/ipe/Kconfig needs to be
updated as well to reflect the makefile change.

-Fan

On Tue, Jan 21, 2025 at 10:58 PM Tyler Hicks <code@tyhicks.com> wrote:
>
> Resolve CONFIG_IPE_BOOT_POLICY relative file paths in the source tree if
> the file was not found within the object tree and is not an absolute path.
>
> This fixes an IPE build failure that occurs when using an output directory,
> such as with the `O=/tmp/build` make option, during a build with the
> CONFIG_IPE_BOOT_POLICY option set to a path that's relative to the kernel
> source tree. For example,
>
>   $ grep CONFIG_IPE_BOOT_POLICY /tmp/build/.config
>   CONFIG_IPE_BOOT_POLICY="ipe-boot-policy"
>   $ touch ipe-boot-policy
>   $ make O=/tmp/build
>   make[1]: Entering directory '/tmp/build'
>     GEN     Makefile
>     UPD     include/config/kernel.release
>     UPD     include/generated/utsrelease.h
>     CALL    scripts/checksyscalls.sh
>     CC      init/version.o
>     AR      init/built-in.a
>     CC      kernel/sys.o
>     AR      kernel/built-in.a
>     IPE_POL ipe-boot-policy
>   An error occurred during policy conversion: : No such file or directory
>   make[5]: *** [security/ipe/Makefile:14: security/ipe/boot_policy.c] Error 2
>   make[4]: *** [scripts/Makefile.build:440: security/ipe] Error 2
>   make[3]: *** [scripts/Makefile.build:440: security] Error 2
>   make[2]: *** [Makefile:1989: .] Error 2
>   make[1]: *** [Makefile:251: __sub-make] Error 2
>   make[1]: Leaving directory '/tmp/build'
>   make: *** [Makefile:251: __sub-make] Error 2
>
> Fixes: ba199dc909a2 ("scripts: add boot policy generation program")
> Cc: stable@vger.kernel.org
> Signed-off-by: Tyler Hicks <code@tyhicks.com>
> ---
>  security/ipe/Makefile | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/security/ipe/Makefile b/security/ipe/Makefile
> index 2ffabfa63fe9..b54d7b7c9e6d 100644
> --- a/security/ipe/Makefile
> +++ b/security/ipe/Makefile
> @@ -10,8 +10,10 @@ quiet_cmd_polgen = IPE_POL $(2)
>
>  targets += boot_policy.c
>
> -$(obj)/boot_policy.c: scripts/ipe/polgen/polgen $(CONFIG_IPE_BOOT_POLICY) FORCE
> -       $(call if_changed,polgen,$(CONFIG_IPE_BOOT_POLICY))
> +boot-pol := $(if $(wildcard $(CONFIG_IPE_BOOT_POLICY)),,$(srctree)/)$(CONFIG_IPE_BOOT_POLICY)
> +
> +$(obj)/boot_policy.c: scripts/ipe/polgen/polgen $(boot-pol) FORCE
> +       $(call if_changed,polgen,$(boot-pol))
>
>  obj-$(CONFIG_SECURITY_IPE) += \
>         boot_policy.o \
> --
> 2.34.1
>
Tyler Hicks Jan. 23, 2025, 1:38 a.m. UTC | #2 
On 2025-01-22 10:41:07, Fan Wu wrote:
> Thanks for the fix.
> 
> My only concern here is the use of wildcard. I'm not sure but if
> $(CONFIG_IPE_BOOT_POLICY) is a glob pattern it could match multiple
> files?

Ah, that's a great point. To be honest, I followed the example of
CONFIG_MODULE_SIG_KEY in scripts/Makefile.modinst and didn't consider this
possible issue.

I can reproduce your concern. It doesn't actually cause any problems because
only the first matched file is used for the boot policy but it is potentially
confusing and boot policy is not something that we want to be unsure about.

Let me think of something else. It might be a few days before I get a chance to
send out a v2.

> Other than that I think the doc of security/ipe/Kconfig needs to be
> updated as well to reflect the makefile change.

Were you thinking something like this?

          This option specifies a filepath to an IPE policy that is compiled
	  into the kernel. The filepath can be absolute or relative from either
	  the source tree or the object tree. This policy will be enforced
	  until a policy update is deployed via the
          $securityfs/ipe/policies/$policy_name/active interface.

          If unsure, leave blank.

I doubt any users actually want a relative path from the object tree but that
has been supported since IPE was initially merged so I think it is worth
keeping around.

Tyler

> 
> -Fan
> 
> On Tue, Jan 21, 2025 at 10:58 PM Tyler Hicks <code@tyhicks.com> wrote:
> >
> > Resolve CONFIG_IPE_BOOT_POLICY relative file paths in the source tree if
> > the file was not found within the object tree and is not an absolute path.
> >
> > This fixes an IPE build failure that occurs when using an output directory,
> > such as with the `O=/tmp/build` make option, during a build with the
> > CONFIG_IPE_BOOT_POLICY option set to a path that's relative to the kernel
> > source tree. For example,
> >
> >   $ grep CONFIG_IPE_BOOT_POLICY /tmp/build/.config
> >   CONFIG_IPE_BOOT_POLICY="ipe-boot-policy"
> >   $ touch ipe-boot-policy
> >   $ make O=/tmp/build
> >   make[1]: Entering directory '/tmp/build'
> >     GEN     Makefile
> >     UPD     include/config/kernel.release
> >     UPD     include/generated/utsrelease.h
> >     CALL    scripts/checksyscalls.sh
> >     CC      init/version.o
> >     AR      init/built-in.a
> >     CC      kernel/sys.o
> >     AR      kernel/built-in.a
> >     IPE_POL ipe-boot-policy
> >   An error occurred during policy conversion: : No such file or directory
> >   make[5]: *** [security/ipe/Makefile:14: security/ipe/boot_policy.c] Error 2
> >   make[4]: *** [scripts/Makefile.build:440: security/ipe] Error 2
> >   make[3]: *** [scripts/Makefile.build:440: security] Error 2
> >   make[2]: *** [Makefile:1989: .] Error 2
> >   make[1]: *** [Makefile:251: __sub-make] Error 2
> >   make[1]: Leaving directory '/tmp/build'
> >   make: *** [Makefile:251: __sub-make] Error 2
> >
> > Fixes: ba199dc909a2 ("scripts: add boot policy generation program")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Tyler Hicks <code@tyhicks.com>
> > ---
> >  security/ipe/Makefile | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/security/ipe/Makefile b/security/ipe/Makefile
> > index 2ffabfa63fe9..b54d7b7c9e6d 100644
> > --- a/security/ipe/Makefile
> > +++ b/security/ipe/Makefile
> > @@ -10,8 +10,10 @@ quiet_cmd_polgen = IPE_POL $(2)
> >
> >  targets += boot_policy.c
> >
> > -$(obj)/boot_policy.c: scripts/ipe/polgen/polgen $(CONFIG_IPE_BOOT_POLICY) FORCE
> > -       $(call if_changed,polgen,$(CONFIG_IPE_BOOT_POLICY))
> > +boot-pol := $(if $(wildcard $(CONFIG_IPE_BOOT_POLICY)),,$(srctree)/)$(CONFIG_IPE_BOOT_POLICY)
> > +
> > +$(obj)/boot_policy.c: scripts/ipe/polgen/polgen $(boot-pol) FORCE
> > +       $(call if_changed,polgen,$(boot-pol))
> >
> >  obj-$(CONFIG_SECURITY_IPE) += \
> >         boot_policy.o \
> > --
> > 2.34.1
> >
Fan Wu Jan. 23, 2025, 5:01 a.m. UTC | #3 
On Wed, Jan 22, 2025 at 5:38 PM Tyler Hicks <code@tyhicks.com> wrote:
>
> On 2025-01-22 10:41:07, Fan Wu wrote:
> > Thanks for the fix.
> >
> > My only concern here is the use of wildcard. I'm not sure but if
> > $(CONFIG_IPE_BOOT_POLICY) is a glob pattern it could match multiple
> > files?
>
> Ah, that's a great point. To be honest, I followed the example of
> CONFIG_MODULE_SIG_KEY in scripts/Makefile.modinst and didn't consider this
> possible issue.
>
> I can reproduce your concern. It doesn't actually cause any problems because
> only the first matched file is used for the boot policy but it is potentially
> confusing and boot policy is not something that we want to be unsure about.
>
> Let me think of something else. It might be a few days before I get a chance to
> send out a v2.
>
> > Other than that I think the doc of security/ipe/Kconfig needs to be
> > updated as well to reflect the makefile change.
>
> Were you thinking something like this?
>
>           This option specifies a filepath to an IPE policy that is compiled
>           into the kernel. The filepath can be absolute or relative from either
>           the source tree or the object tree. This policy will be enforced
>           until a policy update is deployed via the
>           $securityfs/ipe/policies/$policy_name/active interface.
>
>           If unsure, leave blank.
>
Thanks, this looks good to me.

-Fan


> I doubt any users actually want a relative path from the object tree but that
> has been supported since IPE was initially merged so I think it is worth
> keeping around.
>
> Tyler
>
> >
> > -Fan
> >
> > On Tue, Jan 21, 2025 at 10:58 PM Tyler Hicks <code@tyhicks.com> wrote:
> > >
> > > Resolve CONFIG_IPE_BOOT_POLICY relative file paths in the source tree if
> > > the file was not found within the object tree and is not an absolute path.
> > >
> > > This fixes an IPE build failure that occurs when using an output directory,
> > > such as with the `O=/tmp/build` make option, during a build with the
> > > CONFIG_IPE_BOOT_POLICY option set to a path that's relative to the kernel
> > > source tree. For example,
> > >
> > >   $ grep CONFIG_IPE_BOOT_POLICY /tmp/build/.config
> > >   CONFIG_IPE_BOOT_POLICY="ipe-boot-policy"
> > >   $ touch ipe-boot-policy
> > >   $ make O=/tmp/build
> > >   make[1]: Entering directory '/tmp/build'
> > >     GEN     Makefile
> > >     UPD     include/config/kernel.release
> > >     UPD     include/generated/utsrelease.h
> > >     CALL    scripts/checksyscalls.sh
> > >     CC      init/version.o
> > >     AR      init/built-in.a
> > >     CC      kernel/sys.o
> > >     AR      kernel/built-in.a
> > >     IPE_POL ipe-boot-policy
> > >   An error occurred during policy conversion: : No such file or directory
> > >   make[5]: *** [security/ipe/Makefile:14: security/ipe/boot_policy.c] Error 2
> > >   make[4]: *** [scripts/Makefile.build:440: security/ipe] Error 2
> > >   make[3]: *** [scripts/Makefile.build:440: security] Error 2
> > >   make[2]: *** [Makefile:1989: .] Error 2
> > >   make[1]: *** [Makefile:251: __sub-make] Error 2
> > >   make[1]: Leaving directory '/tmp/build'
> > >   make: *** [Makefile:251: __sub-make] Error 2
> > >
> > > Fixes: ba199dc909a2 ("scripts: add boot policy generation program")
> > > Cc: stable@vger.kernel.org
> > > Signed-off-by: Tyler Hicks <code@tyhicks.com>
> > > ---
> > >  security/ipe/Makefile | 6 ++++--
> > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/security/ipe/Makefile b/security/ipe/Makefile
> > > index 2ffabfa63fe9..b54d7b7c9e6d 100644
> > > --- a/security/ipe/Makefile
> > > +++ b/security/ipe/Makefile
> > > @@ -10,8 +10,10 @@ quiet_cmd_polgen = IPE_POL $(2)
> > >
> > >  targets += boot_policy.c
> > >
> > > -$(obj)/boot_policy.c: scripts/ipe/polgen/polgen $(CONFIG_IPE_BOOT_POLICY) FORCE
> > > -       $(call if_changed,polgen,$(CONFIG_IPE_BOOT_POLICY))
> > > +boot-pol := $(if $(wildcard $(CONFIG_IPE_BOOT_POLICY)),,$(srctree)/)$(CONFIG_IPE_BOOT_POLICY)
> > > +
> > > +$(obj)/boot_policy.c: scripts/ipe/polgen/polgen $(boot-pol) FORCE
> > > +       $(call if_changed,polgen,$(boot-pol))
> > >
> > >  obj-$(CONFIG_SECURITY_IPE) += \
> > >         boot_policy.o \
> > > --
> > > 2.34.1
> > >
diff mbox series

Patch

diff --git a/security/ipe/Makefile b/security/ipe/Makefile
index 2ffabfa63fe9..b54d7b7c9e6d 100644
--- a/security/ipe/Makefile
+++ b/security/ipe/Makefile
@@ -10,8 +10,10 @@  quiet_cmd_polgen = IPE_POL $(2)
 
 targets += boot_policy.c
 
-$(obj)/boot_policy.c: scripts/ipe/polgen/polgen $(CONFIG_IPE_BOOT_POLICY) FORCE
-	$(call if_changed,polgen,$(CONFIG_IPE_BOOT_POLICY))
+boot-pol := $(if $(wildcard $(CONFIG_IPE_BOOT_POLICY)),,$(srctree)/)$(CONFIG_IPE_BOOT_POLICY)
+
+$(obj)/boot_policy.c: scripts/ipe/polgen/polgen $(boot-pol) FORCE
+	$(call if_changed,polgen,$(boot-pol))
 
 obj-$(CONFIG_SECURITY_IPE) += \
 	boot_policy.o \