[dm-devel] Fwd: (2) FW: Use-after-free while dm-verity

Gilad Ben-Yossef gilad at benyossef.com
Thu Aug 16 02:53:40 UTC 2018 

I missed sending my reply to the list by myself mistake so forwarding.
---------- Forwarded message ----------
From: Gilad Ben-Yossef <gilad at benyossef.com>
Date: Thu, 16 Aug 2018 09:48:19 +0700
Subject: Re: (2) FW: Use-after-free while dm-verity
To: hyeon.chung at samsung.com

On 8/16/18, 정혜연 <hyeon.chung at samsung.com> wrote:
> Thank you for reply and sorry for inturrupting your vacation.
>

Eh, don't worry about it. I enjoy kernel development work. Just don't
tell my wife... :-)
>
>
> I guess kunmap_atomic / kmap_atomic could be protected from scheduling but
> the operation using the resource like memcopy cannot be protected, right?
>
> Please let me know if I was wrong.

kmap_atomic() disables preemption until the kunmap operation and so
scheduling should not be possible while the memory is mapped.


>
>
>
> I am testing after reverting and resolving confict now. But as I said before
> I'm not sure it is ok just revert only this one.
>
> Is it ok?

The patch stands on its own so that should be ok if that patch is the
root cause of the problem bit if the problem is some where else it
could also be hiding the problem so it would be better to understand
what is going on first or it could bite you elsewhere.

Gilad
>
>
>
> Best Regards,
>
> Hyeyeon Chung
>
>
>
> --------- Original Message ---------
>
> Sender : Gilad Ben-Yossef <gilad at benyossef.com>
>
> Date : 2018-08-16 09:57 (GMT+9)
>
> Title : Re: FW: Use-after-free while dm-verity
>
>
>
> Hi Hyeon,
>
> On Thu, Aug 16, 2018 at 3:35 AM, 정혜연 <hyeon.chung at samsung.com> wrote:
>>
>> Hi,
>>
>>
>>
>> We found some async commit and are wondering if this could be make this
>> issue or not.
>>
>> I want to test after reverting this but I'm not sure it is ok to revert
>> only this commit and when I revert it there's a conflict.
>>
>>
>>
>> Could you please help me?
>
>
> I am on vacation, so please excuse any delay in handling this. Luckily I
> have my laptop with me and will try to do me best to help.
>
> The description below relates to "use-after-free". I just want to make sure
> I understand: does this relate to the memcpy after unmap, right?
>
> Normally, I would say that since we use kmap_atomic to map the data,
> preemption should be disabled and scheduling should not happen, I believe.
>
> I will only be able to take a look at the code in the evening, but in the
> mean time, can you verify the area the memcpy is applied to is mapped with
> kmap_atomic?
>
> thanks!
> Gilad
>
>>
>>
>> commit d1ac3ff008fb9a48f91fc15920b4c8db24c0f03e
>> Author: Gilad Ben-Yossef <gilad at benyossef.com>
>> Date:   Sun Feb 19 14:46:07 2017 +0200
>>
>>     dm verity: switch to using asynchronous hash crypto API
>>
>>     Use of the synchronous digest API limits dm-verity to using pure
>>     CPU based algorithm providers and rules out the use of off CPU
>>     algorithm providers which are normally asynchronous by nature,
>>     potentially freeing CPU cycles.
>>
>>     This can reduce performance per Watt in situations such as during
>>     boot time when a lot of concurrent file accesses are made to the
>>     protected volume.
>>
>>     Signed-off-by: Gilad Ben-Yossef <gilad at benyossef.com>
>>     CC: Eric Biggers <ebiggers3 at gmail.com>
>>     CC: Ondrej Mosn찼훾ek <omosnacek+linux-crypto at gmail.com>
>>     Tested-by: Milan Broz <gmazyland at gmail.com>
>>     Signed-off-by: Mike Snitzer <snitzer at redhat.com>
>>
>>
>>
>>
>>
>> 감사합니다.
>>
>> 정혜연 드림
>>
>>
>>
>>
>>
>> --------- Original Message ---------
>>
>> Sender : 정혜연 <hyeon.chung at samsung.com> Principal
>> Engineer/Platform개발팀(S.LSI)/삼성전자
>>
>> Date : 2018-08-14 17:34 (GMT+9)
>>
>> Title : Use-after-free while dm-verity
>>
>>
>>
>> Hi,
>>
>> I'm a samsung engineer who is responsible for dm-verity.
>>
>>
>>
>> We just enabled dm-verity for android verfied boot 2.0 (using linux
>> 4.14.56).
>>
>> After enabling, we met kernel panic issue caused by dm-verity frequently
>> and it always showed same call stack like below.
>>
>>
>>
>> Could you please let me know If you know any similar issue or have any
>> opinion?
>>
>> If this mail list is not right, please forward proper person or let me
>> know.
>>
>>
>>
>> Thanks in advance.
>>
>>
>>
>> < 4>[ 4686.384679]  [5:  kworker/u16:1:   58] Workqueue: kverityd
>> verity_work
>> < 4>[ 4686.384691]  [5:  kworker/u16:1:   58] task: ffffffc8742a0080
>> task.stack: ffffff800b1d8000
>> < 4>[ 4686.384705]  [5:  kworker/u16:1:   58] PC is at __memcpy+0x70/0x180
>> < 4>[ 4686.384718]  [5:  kworker/u16:1:   58] LR is at
>> crypto_sha1_update+0x94/0xe4
>>
>> ...
>>
>> <4>[ 4686.387488]  [5:  kworker/u16:1:   58] [<ffffff8008b07370>]
>> __memcpy+0x70/0x180
>> < 4>[ 4686.387503]  [5:  kworker/u16:1:   58] [<ffffff80083a0f6c>]
>> crypto_shash_update+0x2c/0x34
>> < 4>[ 4686.387514]  [5:  kworker/u16:1:   58] [<ffffff80083a12c0>]
>> shash_ahash_update+0x3c/0x80
>> < 4>[ 4686.387525]  [5:  kworker/u16:1:   58] [<ffffff80083a1638>]
>> shash_async_update+0x10/0x18
>> < 4>[ 4686.387538]  [5:  kworker/u16:1:   58] [<ffffff8008824974>]
>> verity_hash_update+0x50/0xdc
>> < 4>[ 4686.387549]  [5:  kworker/u16:1:   58] [<ffffff80088247c0>]
>> verity_hash+0x58/0xa0
>> < 4>[ 4686.387560]  [5:  kworker/u16:1:   58] [<ffffff8008824c98>]
>> verity_verify_level+0xc4/0x190
>> < 4>[ 4686.387571]  [5:  kworker/u16:1:   58] [<ffffff8008824bc4>]
>> verity_hash_for_block+0xf0/0x100
>> < 4>[ 4686.387581]  [5:  kworker/u16:1:   58] [<ffffff80088244c8>]
>> fec_read_bufs+0x144/0x30c
>> < 4>[ 4686.387592]  [5:  kworker/u16:1:   58] [<ffffff8008823730>]
>> fec_decode_rsb+0x170/0x4bc
>> < 4>[ 4686.387603]  [5:  kworker/u16:1:   58] [<ffffff8008823524>]
>> verity_fec_decode+0xe8/0x184
>> < 4>[ 4686.387613]  [5:  kworker/u16:1:   58] [<ffffff8008824d38>]
>> verity_verify_level+0x164/0x190
>> < 4>[ 4686.387623]  [5:  kworker/u16:1:   58] [<ffffff8008824bc4>]
>> verity_hash_for_block+0xf0/0x100
>> < 4>[ 4686.387634]  [5:  kworker/u16:1:   58] [<ffffff80088264cc>]
>> verity_work+0xf8/0x308
>> < 4>[ 4686.387646]  [5:  kworker/u16:1:   58] [<ffffff80080cb5ec>]
>> process_one_work+0x2d4/0x608
>> < 4>[ 4686.387656]  [5:  kworker/u16:1:   58] [<ffffff80080cbbf4>]
>> worker_thread+0x220/0x340
>> < 4>[ 4686.387667]  [5:  kworker/u16:1:   58] [<ffffff80080d0498>]
>> kthread+0x11c/0x12c
>> < 4>[ 4686.387678]  [5:  kworker/u16:1:   58] [<ffffff800808557c>]
>> ret_from_fork+0x10/0x18
>> < 0>[ 4686.387690]  [5:  kworker/u16:1:   58] Code: 54000080 540000ab
>> a8c12027 a88120c7 (a8c12027)
>> < 4>[ 4686.387839]  [5:  kworker/u16:1:   58] ---[ end trace
>> 54664349b0f9422c ]---
>>
>>
>>
>>
>>
>> In memcopy function,
>>
>> load x1 register successfully then context switched by scheduler.
>>
>> After returning to same memcopy function, failed another x1 load becuase
>> this is unmapped.(walk->data)
>>
>> <1>[  889.732488]  [2:  kworker/u16:3:15332] Unable to handle kernel
>> paging request at virtual address ffffffc00b6ad000
>>
>>
>>
>> I suspect that this is not protected propely becuase callstack and problem
>> (walk->data: use-after-free) is alwasys same.
>>
>> 1. shrink_node is executed on onther core at that time. Is it
>> possible this make this problem?
>>
>> 2. I found that walk->data is unmapped by ahahs.c but I don't know whether
>> this cause this unmapped situation or not.
>>
>> if (walk->flags & CRYPTO_ALG_ASYNC)
>>     kunmap(walk->pg);
>> else
>>     kunmap_atomic(walk->data);
>>
>>
>>
>>
>>
>> Best Regards,
>>
>> HyeYeon Chung
>>
>>
>>
>>
>>
>> [image]
>
>
>
>
> --
> Gilad Ben-Yossef
> Chief Coffee Drinker
>
> values of β will give rise to dom!
>
>
>
> [image]
>
> [image]


-- 
Gilad Ben-Yossef
Chief Coffee Drinker

values of β will give rise to dom!



-- 
Gilad Ben-Yossef
Chief Coffee Drinker

values of β will give rise to dom!

More information about the dm-devel mailing list