[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: |
Maxim Levitsky |
Subject: |
Re: [PATCH v2 05/11] block/crypto: implement the encryption key management |
Date: |
Fri, 08 Nov 2019 13:04:11 +0200 |
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.
All that patch does is to rename block_crypto_write_func to
block_crypto_create_write_func
and same (for consistency) for block_crypto_init_func ->
block_crypto_create_init_func
And then in this patch I add the block_crypto_write_func, to be used for
anything
but creation code, together with existing block_crypto_read_func which is
already
not used for creation.
Best regards,
Maxim Levitsky