[PATCH 01/18] arch: mach-k3: security: fix the check for authentication

[Manorit Chawdhry]

Hi Andrew,

On 09:22-20230714, Andrew Davis wrote:
> On 7/14/23 12:52 AM, Manorit Chawdhry wrote:
> > Fix regression occurred during refactoring for the mentioned commit.
> > 
> > Fixes: bd6a24759374 ("arm: mach-k3: security: separate out validating binary logic")
> > 
> > Signed-off-by: Manorit Chawdhry <m-chawdhry at ti.com>
> > ---
> >   arch/arm/mach-k3/security.c | 5 +++--
> >   1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/arm/mach-k3/security.c b/arch/arm/mach-k3/security.c
> > index 02a2c12dbd6f..6038c9665ecb 100644
> > --- a/arch/arm/mach-k3/security.c
> > +++ b/arch/arm/mach-k3/security.c
> > @@ -91,8 +91,9 @@ void ti_secure_image_post_process(void **p_image, size_t *p_size)
> >   		return;
> >   	}
> > -	if (get_device_type() != K3_DEVICE_TYPE_HS_SE &&
> > -	    get_device_type() != K3_DEVICE_TYPE_HS_FS)
> > +	if (get_device_type() == K3_DEVICE_TYPE_GP &&
> > +	    (get_device_type() != K3_DEVICE_TYPE_HS_SE &&
> 
> Why check for both == GP and != HS-SE? Seems like this logic could still
> be buggy. Could you explain here exactly what you are trying to do here.
> 

Ah yes, this fix can become better now that I see as there are still
some problems that don't become clear. So basically during the refactor
two things were separated out from this function - 

1. If we are booting signed binary on a GP device then strip off the headers and
   skip the authentication as it is not required on the GP device along
   with a print saying trying to boot signed binary on GP device.

2. If we are on an non-HS device that supports authentication like HS-FS
   or any other device, but we don't have the certificate then we can just
   skip the authentication as other devices don't have anything enforced
   and add a print saying that we are skipping authentication.

Though I don't think the 2nd point should've been part of the refactor
and we only needed the first point to be moved in that function, letting
the 2nd point moved actually causes confusion as we could still get the
print saying that we are skipping authentication but still authenticate
due to the buggy check that was there in this function and the whole
check along with the print should be moved here now that I notice.

Will be fixing this in the next revision.

Thanks and regards,
Manorit

> Andrew
> 
> > +	     !ti_secure_cert_detected(*p_image)))
> >   		return;
> >   	/* Clean out image so it can be seen by system firmware */
> >