From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: ARC-Seal: i=1; a=rsa-sha256; t=1516604845; cv=none; d=google.com; s=arc-20160816; b=bVdfX12d+0mlkla/47lwkrwejWqR2HdQbtw2T+9mlYNCEkgRv8Gt/Yb2wvQ2Z4mqGA spUZU8YnyJUskHeeqYdg2Tf6jQ/4mSluT53VnkvN15/I73fI7xxlhuac29cc0PuWGidy 2xD/5CQmUtZS4ygDj29N7n1el1KATQ5ep2VwD2Y3dsne9WVKv+Dy7ciP1RWlN1SXFKwA RPzqmCAHsMRqR0OE0ADu1PUIqbxuqlVJ0lPPhdWmtHqG5Xwkv+agUaj9jpJiW+wAEVWN xk3vZtitxU80tBI/FCrBebJkUuX+9X0nKKQ6XdQJOU5KrBdjJYNWkZjqnn7M/pHKWYdW QIQg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=cc:to:subject:message-id:date:from:references:in-reply-to :mime-version:dkim-signature:arc-authentication-results; bh=2ORGU1WxkAQQWKaqC+MYGazKHLurZqfq04XpTX3YkOI=; b=kk5B12EvwvNzuI3I4QDsKxcys6u5hnx9V3PzbCrIl/opuI4ACAL2Z1UUZxKyDN5Go6 590cvkUxUxdYNkW9eEddtpHOpFe/lVmymjO2tsmNHS8zUEBgl2CutfXvx24AxqfyOHNs 5V4nNlxCKV0Qvh7JvsmnCGo3tWPyU3ysr7CQ1k6P6wwQ/51G1RjkdINY4eRvYB3JheyQ 88qbXFj6F8EkMDa7Au2OeHXMmvXAVIaYDUt14IJvrYau1+UnrELDyLeLgXYtIJkYtLfp jKnY74WDw/Luo/vcdZDNXZgrB+axKOD/+s94av9rpJp3dnSDYMKPCtKW1aHOKPd8Xarx knbA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@benyossef-com.20150623.gappssmtp.com header.s=20150623 header.b=fo7Id3dm; spf=neutral (google.com: 209.85.220.65 is neither permitted nor denied by best guess record for domain of gilad@benyossef.com) smtp.mailfrom=gilad@benyossef.com Authentication-Results: mx.google.com; dkim=pass header.i=@benyossef-com.20150623.gappssmtp.com header.s=20150623 header.b=fo7Id3dm; spf=neutral (google.com: 209.85.220.65 is neither permitted nor denied by best guess record for domain of gilad@benyossef.com) smtp.mailfrom=gilad@benyossef.com X-Google-Smtp-Source: AH8x225IOEuGF3RiR74I3XDFr3mRSgPuuWOzke4EBWBMgp7sF36sWXKEQrre0HREI5ttjGvKDgBytMYF6eoVW/WOlXc= MIME-Version: 1.0 X-Originating-IP: [217.140.96.140] In-Reply-To: <20180111100137.GA15690@Red> References: <1515662239-1714-1-git-send-email-gilad@benyossef.com> <1515662239-1714-4-git-send-email-gilad@benyossef.com> <20180111100137.GA15690@Red> From: Gilad Ben-Yossef Date: Mon, 22 Jan 2018 09:07:24 +0200 Message-ID: Subject: Re: [PATCH 3/7] crypto: ccree: add ablkcipher support To: Corentin Labbe Cc: Herbert Xu , "David S. Miller" , Greg Kroah-Hartman , Ofir Drang , Linux kernel mailing list , Linux Crypto Mailing List , devel@driverdev.osuosl.org Content-Type: text/plain; charset="UTF-8" X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: =?utf-8?q?1589287079022603421?= X-GMAIL-MSGID: =?utf-8?q?1590275442165310742?= X-Mailing-List: linux-kernel@vger.kernel.org List-ID: Hi Corentin, On Thu, Jan 11, 2018 at 12:01 PM, Corentin Labbe wrote: > On Thu, Jan 11, 2018 at 09:17:10AM +0000, Gilad Ben-Yossef wrote: >> Add CryptoCell ablkcipher support >> > > Hello > > I have some minor comments: > > ablkcipher is deprecated, so you need to use skcipher instead. Fixed in v2. > >> Signed-off-by: Gilad Ben-Yossef >> --- >> drivers/crypto/ccree/Makefile | 2 +- >> drivers/crypto/ccree/cc_buffer_mgr.c | 125 ++++ >> drivers/crypto/ccree/cc_buffer_mgr.h | 10 + >> drivers/crypto/ccree/cc_cipher.c | 1167 ++++++++++++++++++++++++++++++++++ >> drivers/crypto/ccree/cc_cipher.h | 74 +++ >> drivers/crypto/ccree/cc_driver.c | 11 + >> drivers/crypto/ccree/cc_driver.h | 2 + >> 7 files changed, 1390 insertions(+), 1 deletion(-) >> create mode 100644 drivers/crypto/ccree/cc_cipher.c >> create mode 100644 drivers/crypto/ccree/cc_cipher.h >> > [...] >> + >> +struct tdes_keys { >> + u8 key1[DES_KEY_SIZE]; >> + u8 key2[DES_KEY_SIZE]; >> + u8 key3[DES_KEY_SIZE]; >> +}; >> + >> +static const u8 zero_buff[] = { 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, >> + 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, >> + 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, >> + 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}; >> + > > This constant is used nowhere. Fixed in v2. > >> +/* The function verifies that tdes keys are not weak.*/ >> +static int cc_verify_3des_keys(const u8 *key, unsigned int keylen) >> +{ >> + struct tdes_keys *tdes_key = (struct tdes_keys *)key; >> + >> + /* verify key1 != key2 and key3 != key2*/ >> + if ((memcmp((u8 *)tdes_key->key1, (u8 *)tdes_key->key2, >> + sizeof(tdes_key->key1)) == 0) || >> + (memcmp((u8 *)tdes_key->key3, (u8 *)tdes_key->key2, >> + sizeof(tdes_key->key3)) == 0)) { >> + return -ENOEXEC; >> + } >> + >> + return 0; >> +} > > All driver testing 3des key also use des_ekey() Well, the weak key test which is part of des_ekey() are not needed AFAIK for 3des as per RFC2451 and the HW does 3des key expansion so that function is not useful here. > > [...] >> +static void cc_cipher_complete(struct device *dev, void *cc_req, int err) >> +{ >> + struct ablkcipher_request *areq = (struct ablkcipher_request *)cc_req; >> + struct scatterlist *dst = areq->dst; >> + struct scatterlist *src = areq->src; >> + struct blkcipher_req_ctx *req_ctx = ablkcipher_request_ctx(areq); >> + struct crypto_ablkcipher *tfm = crypto_ablkcipher_reqtfm(areq); >> + unsigned int ivsize = crypto_ablkcipher_ivsize(tfm); >> + struct ablkcipher_request *req = (struct ablkcipher_request *)areq; >> + >> + cc_unmap_blkcipher_request(dev, req_ctx, ivsize, src, dst); >> + kfree(req_ctx->iv); > > kzfree for all stuff with IV/key Fixed in v2. > > [...] >> + >> +#ifdef CRYPTO_TFM_REQ_HW_KEY >> + >> +static inline bool cc_is_hw_key(struct crypto_tfm *tfm) >> +{ >> + return (crypto_tfm_get_flags(tfm) & CRYPTO_TFM_REQ_HW_KEY); >> +} >> + >> +#else >> + >> +struct arm_hw_key_info { >> + int hw_key1; >> + int hw_key2; >> +}; >> + >> +static inline bool cc_is_hw_key(struct crypto_tfm *tfm) >> +{ >> + return false; >> +} >> + >> +#endif /* CRYPTO_TFM_REQ_HW_KEY */ > > I see nowhere any use/documentation of CRYPTO_TFM_REQ_HW_KEY, so a cleaning could be done You are right. It's a badly implemented stub function. I'll drop the ifdef as it is useless right now. Many thanks for the review! Gilad -- Gilad Ben-Yossef Chief Coffee Drinker "If you take a class in large-scale robotics, can you end up in a situation where the homework eats your dog?" -- Jean-Baptiste Queru