linux-kernel - Re: [EXT] Re: [PATCH] crypto: caam - use JobR's space to access page 0 regs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <54c5de0c-3c5f-4411-a345-a34852cf9112@pengutronix.de>
Date: Thu, 21 Nov 2024 11:44:17 +0100
From: Ahmad Fatoum <a.fatoum@...gutronix.de>
To: Gaurav Jain <gaurav.jain@....com>, Horia Geanta <horia.geanta@....com>,
 Pankaj Gupta <pankaj.gupta@....com>, Herbert Xu
 <herbert@...dor.apana.org.au>, "David S . Miller" <davem@...emloft.net>,
 Silvano Di Ninno <silvano.dininno@....com>, Varun Sethi <V.Sethi@....com>,
 Meenakshi Aggarwal <meenakshi.aggarwal@....com>,
 Sahil Malhotra <sahil.malhotra@....com>,
 Nikolaus Voss <nikolaus.voss@...g-streit.com>
Cc: "linux-crypto@...r.kernel.org" <linux-crypto@...r.kernel.org>,
 "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
 Pengutronix Kernel Team <kernel@...gutronix.de>
Subject: Re: [EXT] Re: [PATCH] crypto: caam - use JobR's space to access page
 0 regs

Hello Gaurav,

On 18.11.24 10:31, Gaurav Jain wrote:
>> -----Original Message-----
>> From: Ahmad Fatoum <a.fatoum@...gutronix.de>
>> Sent: Monday, November 11, 2024 5:52 PM
>> To: Gaurav Jain <gaurav.jain@....com>; Horia Geanta
>> <horia.geanta@....com>; Pankaj Gupta <pankaj.gupta@....com>; Herbert
>> Xu <herbert@...dor.apana.org.au>; David S . Miller
>> <davem@...emloft.net>; Silvano Di Ninno <silvano.dininno@....com>;
>> Varun Sethi <V.Sethi@....com>; Meenakshi Aggarwal
>> <meenakshi.aggarwal@....com>; Sahil Malhotra
>> <sahil.malhotra@....com>; Nikolaus Voss <nikolaus.voss@...g-streit.com>
>> Cc: linux-crypto@...r.kernel.org; linux-kernel@...r.kernel.org; Pengutronix
>> Kernel Team <kernel@...gutronix.de>
>> Subject: [EXT] Re: [PATCH] crypto: caam - use JobR's space to access page 0
>> regs
>>
>> Caution: This is an external email. Please take care when clicking links or
>> opening attachments. When in doubt, report the message using the 'Report
>> this email' button
>>
>>
>> Hello Guarav,
>>
>> Thanks for your patch.
>>
>> On 11.11.24 13:10, Gaurav Jain wrote:
>>> Access to controller region is not permitted.
>>
>> It's permitted on most of the older SoCs. Please mention on which SoCs this
>> is no longer true and which SoCs you tested your change on.
> Yes, it is permitted on iMX6/7/8M SoCs but not on iMX8DXL/QM/QXP/8ULP.

Ok, please add this to the commit message.

>>
>>> use JobR's register space to access page 0 registers.
>>>
>>> Fixes: 6a83830f649a ("crypto: caam - warn if blob_gen key is
>>> insecure")
>>
>> Did the CAAM even support any of the SoCs, where this doesn't work
>> anymore back when the code was mainlined?
> Yes, for all SECO/ELE based SoCs, CAAM page 0 is not accessible from Non secure world.

Same, this information needs to be in the commit message.

>>
>>> Signed-off-by: Gaurav Jain <gaurav.jain@....com>
>>> ---
>>>  drivers/crypto/caam/blob_gen.c | 3 ++-
>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/crypto/caam/blob_gen.c
>>> b/drivers/crypto/caam/blob_gen.c index 87781c1534ee..079a22cc9f02
>>> 100644
>>> --- a/drivers/crypto/caam/blob_gen.c
>>> +++ b/drivers/crypto/caam/blob_gen.c
>>> @@ -2,6 +2,7 @@
>>>  /*
>>>   * Copyright (C) 2015 Pengutronix, Steffen Trumtrar
>> <kernel@...gutronix.de>
>>>   * Copyright (C) 2021 Pengutronix, Ahmad Fatoum
>>> <kernel@...gutronix.de>
>>> + * Copyright 2024 NXP
>>>   */
>>>
>>>  #define pr_fmt(fmt) "caam blob_gen: " fmt @@ -104,7 +105,7 @@ int
>>> caam_process_blob(struct caam_blob_priv *priv,
>>>       }
>>>
>>>       ctrlpriv = dev_get_drvdata(jrdev->parent);
>>> -     moo = FIELD_GET(CSTA_MOO, rd_reg32(&ctrlpriv->ctrl-
>>> perfmon.status));
>>> +     moo = FIELD_GET(CSTA_MOO,
>>> + rd_reg32(&ctrlpriv->jr[0]->perfmon.status));
>>
>> I believe your change is correct, but I would prefer that ctrlpriv gets a
>> perfmon member that is initialized in caam_probe to either &ctrlpriv->ctrl-
>>> perfmon.status or &ctrlpriv->jr[0]->perfmon.status and then the code here
>> would just use &ctrlpriv->perfmon->status.
>>
>> This would simplify code not only here, but also in caam_ctrl_rng_init.
> As already communicated by Horia, a separate patch is good to cover this.

Are you interested in writing that separate patch?

Cheers,
Ahmad

> 
> Thanks
> Gaurav
>>
>> Thanks,
>> Ahmad
>>
>>
>>>       if (moo != CSTA_MOO_SECURE && moo != CSTA_MOO_TRUSTED)
>>>               dev_warn(jrdev,
>>>                        "using insecure test key, enable HAB to use
>>> unique device key!\n");
>>
>>
>> --
>> Pengutronix e.K.                           |                             |
>> Steuerwalder Str. 21                       |
>> https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.
>> pengutronix.de%2F&data=05%7C02%7Cgaurav.jain%40nxp.com%7C758768
>> 98a8044b366f4808dd024b7740%7C686ea1d3bc2b4c6fa92cd99c5c30163
>> 5%7C0%7C0%7C638669245367988594%7CUnknown%7CTWFpbGZsb3d8e
>> yJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIj
>> oiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=aaQ65iMsvuHn3q
>> 0bo5UU%2FYU7Fpyw3El7wNVHd%2BMNee0%3D&reserved=0  |
>> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
>> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> 


-- 
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 |