On 01/28/2016 09:09 AM, Stephan Mueller wrote:
> Am Donnerstag, 28. Januar 2016, 08:00:25 schrieb Tadeusz Struk:
>
> Hi Tadeusz,
>
>> Hi Stephan,
>>
>> On 01/27/2016 10:26 PM, Stephan Mueller wrote:
>>>> + for (i = 0; i < areq->tsgls; i++)
>>>>
>>>>> + put_page(sg_page(sg + i));
>>>
>>> Shouldn't here be the same logic as in put_sgl? I.e.
>>>
>>> for (i = 0; i < sgl->cur; i++) {
>>>
>>> if (!sg_page(sg + i))
>>>
>>> continue;
>>>
>>> put_page(sg_page(sg + i));
>>> sg_assign_page(sg + i, NULL);
>>>
>>> }
>>
>> Thanks for reviewing.
>> I don't think it is possible that there ever will be any gaps in the tsgl.
>> In fact if there is such a possibility then it is a serious problem, because
>> it would mean that we are sending NULL ptrs to the ciphers (see line 640):
>>
>> sg_mark_end(sgl->sg + sgl->cur - 1);
>> aead_request_set_crypt(&ctx->aead_req, sgl->sg, ctx-
>> first_rsgl.sgl.sg,
>> used, ctx->iv);
>>
>> I don't see any implementation checking for null in sgls. Most of them just
>> do:
>>
>> for_each_sg(sgl, sg, nents, i)
>> sg_virt(sg)...
>>
>> So it would Oops there. I think this check in put_sgl is redundant.
>> Thanks,
>
> algif_skcipher does a similar check in skcipher_pull_sgl:
>
> ...
> if (!sg_page(sg + i))
> continue;
> ...
> if (put)
> put_page(sg_page(sg + i));
> sg_assign_page(sg + i, NULL);
> ...
>
Yes, that's true, but this is because if you look at the skcipher_recvmsg_async()
function, it invokes crypt operation for each recv segment separately, and after each
iteration advances the tsgl forward and marks the sgs that are already processed with NULL.
This is completely different to the aead use case, which always sends everything
in one go.
Thanks,
--
TS
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
