Re: cbc mode broken in rk3288 driver

[Elon Zhang <zhangzj@rock-chips.com>]

On 8/23/2019 15:33, Ard Biesheuvel wrote:
> On Fri, 23 Aug 2019 at 10:10, Elon Zhang <zhangzj@rock-chips.com> wrote:
>> Hi Ard,
>>
>> I will try to fix this bug.
> Good
>
>> Furthermore, I will submit a patch to  set
>> crypto node default disable in rk3288.dtsi.
>>
> Please don't. The ecb mode works fine, and 'fixing' the DT only helps
> if you use the one that ships with the kernel, which is not always the
> case.
>
But crypto node default 'okay' in SoC dtsi is not good since not all 
boards need this

hardware function. It is better that default 'disbale' in SoC dtsi and 
enabled in specific

board dts.

>
>> On 8/20/2019 23:45, Ard Biesheuvel wrote:
>>> Hello all,
>>>
>>> While playing around with the fuzz tests on kernelci.org (which has a
>>> couple of rk3288 based boards for boot testing), I noticed that the
>>> rk3288 cbc mode driver is still broken (both AES and DES fail).
>>>
>>> For instance, one of the runs failed with
>>>
>>>    alg: skcipher: cbc-aes-rk encryption test failed (wrong result) on
>>> test vector \"random: len=6848 klen=32\", cfg=\"random: may_sleep
>>> use_digest src_divs=[93.41%@+1655, 2.19%@+3968, 4.40%@+22]\"
>>>
>>> (but see below for the details of a few runs)
>>>
>>> However, more importantly, it looks like the driver violates the
>>> scatterlist API, by assuming that sg entries are always mapped and
>>> that sg_virt() and/or page_address(sg_page()) can always be called on
>>> arbitrary scatterlist entries
>>>
>>> The failures in question all occur with inputs whose size > PAGE_SIZE,
>>> so it looks like the PAGE_SIZE limit is interacting poorly with the
>>> way the next IV is obtained.
>>>
>>> Broken CBC is a recipe for disaster, and so this should really be
>>> fixed, or the driver disabled.
>>>
>>
>
>