Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752741AbdIFKOc (ORCPT ); Wed, 6 Sep 2017 06:14:32 -0400 Received: from mail-oi0-f47.google.com ([209.85.218.47]:36740 "EHLO mail-oi0-f47.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752585AbdIFKO1 (ORCPT ); Wed, 6 Sep 2017 06:14:27 -0400 X-Google-Smtp-Source: ADKCNb4M8dRwwnCjYqaHKXQFRnOspsb0vBTtsB21fNxsgnjt0K1Suw9wWuH6LUnVVpV59kPH3T309U98Xe008mBoOC8= MIME-Version: 1.0 X-Originating-IP: [82.81.71.2] In-Reply-To: References: <20170602122446.2427-1-david@sigma-star.at> <20170628132710.97278-1-david@sigma-star.at> From: Gilad Ben-Yossef Date: Wed, 6 Sep 2017 13:14:26 +0300 Message-ID: Subject: Re: [PATCH] crypto: caam - properly set IV after {en,de}crypt To: =?UTF-8?Q?Horia_Geant=C4=83?= Cc: David Gstir , Dan Douglass , "herbert@gondor.apana.org.au" , "davem@davemloft.net" , "richard@sigma-star.at" , "linux-crypto@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "stable@vger.kernel.org" Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by nfs id v86AEcDc019855 Content-Length: 2982 Lines: 69 On Tue, Sep 5, 2017 at 6:33 PM, Horia Geantă wrote: > On 8/14/2017 10:59 AM, Gilad Ben-Yossef wrote: >> Hi, >> >> On Thu, Jun 29, 2017 at 1:19 PM, Horia Geantă wrote: >>> On 6/28/2017 4:42 PM, Horia Geantă wrote: >>>> On 6/28/2017 4:27 PM, David Gstir wrote: >>>>> Certain cipher modes like CTS expect the IV (req->info) of >>>>> ablkcipher_request (or equivalently req->iv of skcipher_request) to >>>>> contain the last ciphertext block when the {en,de}crypt operation is done. >>>>> This is currently not the case for the CAAM driver which in turn breaks >>>>> e.g. cts(cbc(aes)) when the CAAM driver is enabled. >>>>> >>>>> This patch fixes the CAAM driver to properly set the IV after the >>>>> {en,de}crypt operation of ablkcipher finishes. >>>>> >>>>> This issue was revealed by the changes in the SW CTS mode in commit >>>>> 0605c41cc53ca ("crypto: cts - Convert to skcipher") >>>>> >>>>> Cc: # 4.8+ >>>>> Signed-off-by: David Gstir >>>> Reviewed-by: Horia Geantă >>>> >>> Btw, instead of updating the IV in SW, CAAM engine could be programmed >>> to do it - by saving the Context Register of the AES accelerator. >>> >>> Unfortunately this would require changes in quite a few places: shared >>> descriptor, HW S/G generation logic, IV dma (un)mapping and maybe others. >>> >>> So it's better to have this fix now (which, considering size, is >>> appropriate for -stable) and later, if needed, offload IV updating in HW. >>> >> >> My apologies for reviving this thread from the dead, but doesn't the patch fail >> for in-place decryption since we are copying from req->dst after >> the operation is done, and therefore it no longer contains the ciphertext? >> > You are right, thanks! Will follow up with a fix. > Though cts(cbc(aes)) in particular is working, see below. > >> I'm asking since I ran into a similar issue in the ccree driver and thought >> to deploy a similar fix but could not convince myself why this works. >> > IIUC cts(cbc(aes)) in-place decryption (with cbc(aes) offloaded to CAAM > engine) works since SW implementation of cts, when performing the > ciphertext stealing phase in cts_cbc_decrypt() does not use req->iv, but > a previously value, saved before staring decryption in crypto_cts_decrypt(): > > if (cbc_blocks <= 1) > memcpy(space, req->iv, bsize); > else > scatterwalk_map_and_copy(space, req->src, offset - 2 * bsize, > bsize, 0); > Is that not a performance bug in software CTS than? I mean all those transformation drivers doing that extra copy and possibly malloc and free to save the data for the info and than have the CTS implementation ignore that and do its own memory copy? 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