Re: [PATCH master] optee: don't warn about missing OP-TEE header

[Marco Felsch <m.felsch@pengutronix.de>]

On 24-02-29, Ahmad Fatoum wrote:
> On 29.02.24 10:10, Marco Felsch wrote:
> > Hi Ahmad,
> > 
> > On 24-02-28, Ahmad Fatoum wrote:
> >> OP-TEE header is checked once in PBL, saved into scratch area after
> >> verification and then checked again in barebox proper.
> >>
> >> The check in PBL fails silently, but the check in barebox proper that
> >> should always follow, because the header isn't written to the scratch
> >> area is printed with error log level.
> >>
> >> Printing an error in either case is wrong though as using a raw OP-TEE
> >> binary without header is a valid use case and the OP-TEE header may
> >> also be missing when barebox is chainloaded from a running barebox.
> >>
> >> Therefore reduce the message to debug log level.
> >>
> >> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> >> ---
> >>  common/optee.c | 4 ++--
> >>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/common/optee.c b/common/optee.c
> >> index 34667f1f51e0..a8a43554e757 100644
> >> --- a/common/optee.c
> >> +++ b/common/optee.c
> >> @@ -14,8 +14,8 @@ int optee_verify_header(const struct optee_header *hdr)
> >>  		return -EINVAL;
> > 
> > Shouldn't be the fix:
> > 
> > 	if (IS_ERR_OR_NULL(hdr))
> > 		return -EINVAL;
> > 
> > to fail silently.
> 
> hdr is a valid pointer for me, but it doesn't point at a header, which causes
> me to get an error message.

Yes, I have noticed the code path for non-pbl part as well now :/

I was thinking about:

	if (hdr->magic == 0)
		return -EINVAL;

but this is not far awways from you change. Therefore:

Reviewed-by: Marco Felsch <m.felsch@pengutronix.de>

PS: Could you please fix the silent check as well by using IS_ERR_OR_NULL()?

Regards,
  Marco

> 
> Thanks,
> Ahmad
> 
> > 
> > Regards,
> >   Marco
> > 
> >>  
> >>  	if (hdr->magic != OPTEE_MAGIC) {
> >> -		pr_err("Invalid header magic 0x%08x, expected 0x%08x\n",
> >> -			   hdr->magic, OPTEE_MAGIC);
> >> +		pr_debug("Invalid header magic 0x%08x, expected 0x%08x\n",
> >> +			 hdr->magic, OPTEE_MAGIC);
> >>  		return -EINVAL;
> >>  	}
> >>  
> >> -- 
> >> 2.39.2
> >>
> >>
> > 
> 
> -- 
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> 
>