linux-kernel - Re: [EXT] [PATCH v6 3/4] crypto: caam - add in-kernel interface for blob generator

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]

Date:   Tue, 22 Mar 2022 10:37:02 +0100
From:   Ahmad Fatoum <a.fatoum@...gutronix.de>
To:     Pankaj Gupta <pankaj.gupta@....com>,
        Horia Geanta <horia.geanta@....com>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        "David S. Miller" <davem@...emloft.net>
Cc:     Sumit Garg <sumit.garg@...aro.org>,
        David Gstir <david@...ma-star.at>,
        Matthias Schiffer <matthias.schiffer@...tq-group.com>,
        "kernel@...gutronix.de" <kernel@...gutronix.de>,
        Franck Lenormand <franck.lenormand@....com>,
        Richard Weinberger <richard@....at>,
        Jan Luebbe <j.luebbe@...gutronix.de>,
        James Morris <jmorris@...ei.org>,
        Mimi Zohar <zohar@...ux.ibm.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Eric Biggers <ebiggers@...nel.org>,
        "linux-security-module@...r.kernel.org" 
        <linux-security-module@...r.kernel.org>,
        "keyrings@...r.kernel.org" <keyrings@...r.kernel.org>,
        "linux-crypto@...r.kernel.org" <linux-crypto@...r.kernel.org>,
        David Howells <dhowells@...hat.com>,
        Jarkko Sakkinen <jarkko@...nel.org>,
        "linux-integrity@...r.kernel.org" <linux-integrity@...r.kernel.org>,
        James Bottomley <jejb@...ux.ibm.com>,
        "tharvey@...eworks.com" <tharvey@...eworks.com>,
        "Serge E. Hallyn" <serge@...lyn.com>
Subject: Re: [EXT] [PATCH v6 3/4] crypto: caam - add in-kernel interface for
 blob generator

Hello Pankaj,

On 22.03.22 08:32, Ahmad Fatoum wrote:
> Hello Pankaj,
> 
> On 22.03.22 07:25, Pankaj Gupta wrote:
>> Hi Ahmad,
>>
>> Suggested to define macro with more details.
>> Please find comments in-line.
>>
> 
>> len = 4 + (4 + ALIGN(keymod_len, 4)) + 2*(4 + 4 + 
>>>>>> + CAAM_PTR_SZ_MAX) + 4;
>>
>>> +/* header + (key mod immediate) + 2x seq_intlen pointers + op */
>>> +#define CAAM_BLOB_DESC_BYTES_MAX \
>>> +       (CAAM_CMD_SZ + \
>>> +        CAAM_CMD_SZ + CAAM_BLOB_KEYMOD_LENGTH + \
>>> +        2 * (CAAM_CMD_SZ + CAAM_PTR_SZ_MAX) + \
>>> +        CAAM_CMD_SZ)
>>> +
>>
>> Suggested to replace the above macro like below:
>>
>> +#define CAAM_BLOB_DESC_BYTES_MAX \			
>> +       (CAAM_CMD_SZ + \					/* Command to initialize & stating length of  descriptor */
>> +        CAAM_CMD_SZ + CAAM_BLOB_KEYMOD_LENGTH + \	/* Command to append the key-modifier + followed by the key-modifier data */
>> +        (CAAM_CMD_SZ + CAAM_PTR_SZ_MAX) + \		/* Command to include input plain key and pointer to the input key */
>> +        (CAAM_CMD_SZ + CAAM_PTR_SZ_MAX) + \		/* Command to include output-key blob and pointer to the output-key blob */
>> +        CAAM_CMD_SZ)						/* Command describing the Operation to perform */
> 
> 
> Sure thing, will do for v7. Otherwise, if all looks good to you,
> can I have your Reviewed-by?
This doesn't compile as-is and it leads to quite long lines.
The description isn't accurate also, because what's plain and what's blob
changes depending on whether we encapsulate or decapsulate.

Here's my revised macro version:

#define CAAM_BLOB_DESC_BYTES_MAX                                        \
        /* Command to initialize & stating length of descriptor */      \
        (CAAM_CMD_SZ +                                                  \
        /* Command to append the key-modifier + key-modifier data */    \
         CAAM_CMD_SZ + CAAM_BLOB_KEYMOD_LENGTH +                        \
        /* Command to include input key + pointer to the input key */   \
         CAAM_CMD_SZ + CAAM_PTR_SZ_MAX +                                \
        /* Command to include output key + pointer to the output key */ \
         CAAM_CMD_SZ + CAAM_PTR_SZ_MAX +                                \
        /* Command describing the Operation to perform */               \
         CAAM_CMD_SZ)

Alternatively, I can change it back into a function:

static inline u32 *caam_blob_desc_alloc(void)
{
        size_t size = 0;

        /* Command to initialize & stating length of descriptor */
        size += CAAM_CMD_SZ;
        /* Command to append the key-modifier + key-modifier data */
        size += CAAM_CMD_SZ + CAAM_BLOB_KEYMOD_LENGTH;
        /* Command to include input plain key + pointer to the input key */
        size += CAAM_CMD_SZ + CAAM_PTR_SZ_MAX;
        /* Command to include output-key blob + pointer to the output key */
        size += CAAM_CMD_SZ + CAAM_PTR_SZ_MAX;
        /* Command describing the Operation to perform */
        size += CAAM_CMD_SZ;

        return kzalloc(size, GFP_KERNEL | GFP_DMA);
}

Let me know what works better for you.

Cheers,
Ahmad

> 
> Thanks,
> Ahmad
> 


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