On Wed, Aug 23, 2017 at 10:47 AM, Stephan Mueller <smueller@xxxxxxxxxx> wrote:
> Am Mittwoch, 23. August 2017, 09:41:11 CEST schrieb Gilad Ben-Yossef:
>
> Hi Gilad,
>
>>
>> if (areq) {
>> + /*
>> + * The crypto API expects us to set the req->info to the last
>> + * ciphertext block. For encrypt, simply copy from the result.
>> + * For decrypt, we must copy from a saved buffer since this
>> + * could be an in-place decryption operation and the src is
>> + * lost by this point.
>> + */
>> + if (req_ctx->gen_ctx.op_type == DRV_CRYPTO_DIRECTION_DECRYPT) {
>> + memcpy(req->info, req_ctx->backup_info, ivsize);
>> + kfree(req_ctx->backup_info);
>> + } else {
>> + scatterwalk_map_and_copy(req->info, req->dst,
>> + (req->nbytes - ivsize),
>> + ivsize, 0);
>> + }
>> +
>> ablkcipher_request_complete(areq, completion_error);
>> return 0;
>> }
>> @@ -858,7 +876,6 @@ static int ssi_ablkcipher_encrypt(struct
>> ablkcipher_request *req) struct blkcipher_req_ctx *req_ctx =
>> ablkcipher_request_ctx(req); unsigned int ivsize =
>> crypto_ablkcipher_ivsize(ablk_tfm);
>>
>> - req_ctx->backup_info = req->info;
>> req_ctx->is_giv = false;
>>
>> return ssi_blkcipher_process(tfm, req_ctx, req->dst, req->src,
>> req->nbytes, req->info, ivsize, (void *)req, DRV_CRYPTO_DIRECTION_ENCRYPT);
>> @@ -871,8 +888,18 @@ static int ssi_ablkcipher_decrypt(struct
>> ablkcipher_request *req) struct blkcipher_req_ctx *req_ctx =
>> ablkcipher_request_ctx(req); unsigned int ivsize =
>> crypto_ablkcipher_ivsize(ablk_tfm);
>>
>> - req_ctx->backup_info = req->info;
>> + /*
>> + * Allocate and save the last IV sized bytes of the source, which will
>> + * be lost in case of in-place decryption and might be needed for CTS.
>> + */
>> + req_ctx->backup_info = kmalloc(ivsize, GFP_KERNEL);
>> + if (!req_ctx->backup_info)
>> + return -ENOMEM;
>
> Are you sure you do not add a memleak here? You seem to unconditionally
> allocate memory but conditionally free it.
You are right. I've missed freeing the memory on error code paths.
Thank you for catching that.
The code involved is badly need of a re-write but I wanted to get the
obvious bug fixed first.
I will send a revised patch
Thanks,
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
