[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 05/11] block/crypto: implement the encryption key manageme
From: |
Max Reitz |
Subject: |
Re: [PATCH v2 05/11] block/crypto: implement the encryption key management |
Date: |
Fri, 8 Nov 2019 14:12:36 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.1.1 |
On 08.11.19 12:04, Maxim Levitsky wrote:
> On Fri, 2019-11-08 at 11:49 +0100, Max Reitz wrote:
>> On 08.11.19 10:30, Maxim Levitsky wrote:
>>> On Fri, 2019-10-04 at 20:41 +0200, Max Reitz wrote:
>>>> On 13.09.19 00:30, Maxim Levitsky wrote:
>>>>> This implements the encryption key management
>>>>> using the generic code in qcrypto layer
>>>>> (currently only for qemu-img amend)
>>>>>
>>>>> This code adds another 'write_func' because the initialization
>>>>> write_func works directly on the underlying file,
>>>>> because during the creation, there is no open instance
>>>>> of the luks driver, but during regular use, we have it,
>>>>> and should use it instead.
>>>>>
>>>>>
>>>>> This commit also adds a 'hack/workaround' I and Kevin Wolf (thanks)
>>>>> made to make the driver still support write sharing,
>>>>> but be safe against concurrent metadata update (the keys)
>>>>> Eventually write sharing for luks driver will be deprecated
>>>>> and removed together with this hack.
>>>>>
>>>>> The hack is that we ask (as a format driver) for
>>>>> BLK_PERM_CONSISTENT_READ always
>>>>> (technically always unless opened with BDRV_O_NO_IO)
>>>>>
>>>>> and then when we want to update the keys, we
>>>>> unshare that permission. So if someone else
>>>>> has the image open, even readonly, this will fail.
>>>>>
>>>>> Also thanks to Daniel Berrange for the variant of
>>>>> that hack that involves asking for read,
>>>>> rather that write permission
>>>>>
>>>>> Signed-off-by: Maxim Levitsky <address@hidden>
>>>>> ---
>>>>> block/crypto.c | 118 +++++++++++++++++++++++++++++++++++++++++++++++--
>>>>> 1 file changed, 115 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/block/crypto.c b/block/crypto.c
>>>>> index a6a3e1f1d8..f42fa057e6 100644
>>>>> --- a/block/crypto.c
>>>>> +++ b/block/crypto.c
>>>>> @@ -36,6 +36,7 @@ typedef struct BlockCrypto BlockCrypto;
>>>>>
>>>>> struct BlockCrypto {
>>>>> QCryptoBlock *block;
>>>>> + bool updating_keys;
>>>>> };
>>>>>
>>>>>
>>>>> @@ -70,6 +71,24 @@ static ssize_t block_crypto_read_func(QCryptoBlock
>>>>> *block,
>>>>> return ret;
>>>>> }
>>>>>
>>>>> +static ssize_t block_crypto_write_func(QCryptoBlock *block,
>>>>> + size_t offset,
>>>>> + const uint8_t *buf,
>>>>> + size_t buflen,
>>>>> + void *opaque,
>>>>> + Error **errp)
>>>>
>>>> There’s already a function of this name for creation.
>>>
>>> There is a long story why two write functions are needed.
>>> i tried to use only one, but at the end I and Daniel both agreed
>>> that its just better to have two functions.
>>>
>>> The reason is that during creation, the luks BlockDriverState doesn't exist
>>> yet,
>>> and so the creation routine basically just writes to the underlying
>>> protocol driver.
>>>
>>> Thats is why the block_crypto_create_write_func receives a BlockBackend
>>> pointer,
>>> to which the BlockDriverState of the underlying protocol driver is inserted.
>>>
>>>
>>> On the other hand, for amend, the luks block device is open, and it only
>>> knows
>>> about its own BlockDriverState, and thus the io should be done on bs->file
>>>
>>> So instead of trying to coerce a single callback to do both of this,
>>> we decided to just have a little code duplication.
>>
>> I meant: This doesn’t compile. There’s already another function of this
>> name.
>>
>
> You probably didn't apply the 'block-crypto: misc refactoring' patch,
> or I forgot to send it.
Maybe you forgot to mention anywhere that I should.
Max
signature.asc
Description: OpenPGP digital signature