All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Vincent Stehlé" <vincent.stehle@arm.com>
To: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Sergey Matyukevich <geomatsi@gmail.com>, buildroot@buildroot.org
Subject: Re: [Buildroot] [PATCH] boot/arm-trusted-firmware: add patch to fix fiptool link
Date: Fri, 21 Jul 2023 14:58:29 +0200	[thread overview]
Message-ID: <ZLqAtjeeukVSdN4b@debian> (raw)
In-Reply-To: <20230720232827.09c3094d@windsurf>

On Thu, Jul 20, 2023 at 11:28:27PM +0200, Thomas Petazzoni wrote:
> Hello Vincent,

Hi Thomas,

Thanks for the review.

> 
> On Wed, 19 Jul 2023 14:53:09 +0200
> Vincent Stehlé <vincent.stehle@arm.com> wrote:
> 
> > When building a fip firmware (BR2_TARGET_ARM_TRUSTED_FIRMWARE_FIP=y), the
> > TF-A build recipe starts by building the host program fiptool with the
> > proper build environment variables. Then the main TF-A target firmware
> > build step takes place, with the expectation that the fiptool program will
> > be used under the hood if necessary.
> > 
> > In TF-A, the build recipe for the host program fiptool has subtly changed
> > after v2.7, in commit cf2dd17ddda2 ("refactor(security): add OpenSSL 1.x
> > compatibility"). This change has the effect to force re-linking fiptool
> > each time.
> > 
> > If we try to build with Buildroot a fip firmware with a TF-A version after
> > v2.7 comprising the aforementioned change, the fiptool program is forcibly
> > re-linked during the main firmware build step. This happens without the
> > proper build environment variables and consequently, if openssl is not
> > installed on the host, the libcrypto shared library will not be found by
> > the linker and the link will fail with the following error:
> > 
> >   /usr/bin/ld: cannot find -lcrypto: No such file or directory
> > 
> > A patch has been integrated into TF-A to avoid re-linking fiptool when not
> > necessary, which should solve the problem starting with version v2.10. Add
> > that patch in Buildroot for versions v2.8 and v2.9, to repair the build in
> > the cases described above.
> > 
> > Signed-off-by: Vincent Stehlé <vincent.stehle@arm.com>
> > Cc: Dick Olsson <hi@senzilla.io>
> > Cc: Sergey Matyukevich <geomatsi@gmail.com>
> > ---
> > 
> > 
> > Hi,
> > 
> > This can be tested with e.g. tests.boot.test_edk2 in an environment with no
> > openssl (libcrypto) installed.
> 
> Indeed, which means this commit can have:
> 
> Fixes:
> 
>   https://gitlab.com/buildroot.org/buildroot/-/jobs/4664845767
> 

Ok, thanks. I will send a v2 with this mention.

> As usual, one concern is that your commit is fixing the issue for
> people using v2.8 and v2.9 precisely, but anyone using a custom version
> based on 2.8 or 2.9 will not benefit from this fix.

You are right, this solution is not perfect.

To mitigate this bad impression, here is a quick audit of the existing
defconfigs in the tree:

  45 have TF-A
   9 have TF-A with FIP
   2 have a custom TF-A version (v2.4-stm32mp-r1)
   2 have a TF-A version in v2.8..v2.9
   0 have TF-A with FIP and a custom version in v2.8..v2.9

I would say that for what is in tree at least we are safe.

> 
> One option would be to pass the relevant environment variables also at
> build time, so that when fiptool is relinked, it gets relinked
> correctly. But it's quite annoying to keep this pretty much forever in
> Buildroot. So perhaps your solution with a patch is the most
> reasonable, and people affected by the issue will find the patch in
> Buildroot and use it for their custom version as well.

Another approach might be for the makefile to apply the patch only if it can
determine that the TF-A version is in the range v2.8..v2.9 and that fiptool is
necessary.
I think it is nearly impossible to make the range determination work in the
general case (for example when the version given is a commit id), therefore I
did not investigate that path seriously.

Best regards,
Vincent.

> 
> Thomas
> -- 
> Thomas Petazzoni, co-owner and CEO, Bootlin
> Embedded Linux and Kernel engineering and training
> https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

  reply	other threads:[~2023-07-21 12:58 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-19 12:53 [Buildroot] [PATCH] boot/arm-trusted-firmware: add patch to fix fiptool link Vincent Stehlé
2023-07-20 21:28 ` Thomas Petazzoni via buildroot
2023-07-21 12:58   ` Vincent Stehlé [this message]
2023-07-21 13:09 ` [Buildroot] [PATCH v2] " Vincent Stehlé
2023-07-23 17:33   ` Yann E. MORIN
2023-08-30  6:13   ` Peter Korsgaard

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZLqAtjeeukVSdN4b@debian \
    --to=vincent.stehle@arm.com \
    --cc=buildroot@buildroot.org \
    --cc=geomatsi@gmail.com \
    --cc=thomas.petazzoni@bootlin.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.