On Thu, Dec 06, 2018 at 05:42:54PM +0000, Vladimir Sementsov-Ogievskiy wrote:
> 06.12.2018 13:54, Daniel P. Berrangé wrote:
> > On Wed, Dec 05, 2018 at 05:47:00PM +0300, Vladimir Sementsov-Ogievskiy
> > wrote:
> >> The two thing that should be handled are cipher and ivgen. For ivgen
> >> the solution is just mutex, as iv calculations should not be long in
> >> comparison with encryption/decryption. And for cipher let's just keep
> >> per-thread ciphers.
> >>
> >> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>
> >> ---
>
> [..]
>
> >> --- a/crypto/block.c
> >> +++ b/crypto/block.c
>
> [..]
>
> >> static int do_qcrypto_cipher_encrypt(QCryptoCipher *cipher,
> >> size_t niv,
> >> QCryptoIVGen *ivgen,
> >> + QemuMutex *ivgen_mutex,
> >> int sectorsize,
> >> uint64_t offset,
> >> uint8_t *buf,
> >> @@ -218,10 +307,15 @@ static int do_qcrypto_cipher_encrypt(QCryptoCipher
> >> *cipher,
> >> while (len > 0) {
> >> size_t nbytes;
> >> if (niv) {
> >> - if (qcrypto_ivgen_calculate(ivgen,
> >> - startsector,
> >> - iv, niv,
> >> - errp) < 0) {
> >> + if (ivgen_mutex) {
> >> + qemu_mutex_lock(ivgen_mutex);
> >> + }
> >> + ret = qcrypto_ivgen_calculate(ivgen, startsector, iv, niv,
> >> errp);
> >> + if (ivgen_mutex) {
> >> + qemu_mutex_unlock(ivgen_mutex);
> >> + }
> >
> > I think we should just make ivgen_mutex compulsory....
> >
> >> +
> >> + if (ret < 0) {
> >> goto cleanup;
> >> }
> >>
> >> @@ -258,8 +352,9 @@ int qcrypto_cipher_decrypt_helper(QCryptoCipher
> >> *cipher,
> >> size_t len,
> >> Error **errp)
> >> {
> >> - return do_qcrypto_cipher_encrypt(cipher, niv, ivgen, sectorsize,
> >> offset,
> >> - buf, len, qcrypto_cipher_decrypt,
> >> errp);
> >> + return do_qcrypto_cipher_encrypt(cipher, niv, ivgen, NULL, sectorsize,
> >> + offset, buf, len,
> >> qcrypto_cipher_decrypt,
> >> + errp);
> >> }
> >>
> >>
> >> @@ -272,11 +367,11 @@ int qcrypto_cipher_encrypt_helper(QCryptoCipher
> >> *cipher,
> >> size_t len,
> >> Error **errp)
> >> {
> >> - return do_qcrypto_cipher_encrypt(cipher, niv, ivgen, sectorsize,
> >> offset,
> >> - buf, len, qcrypto_cipher_encrypt,
> >> errp);
> >> + return do_qcrypto_cipher_encrypt(cipher, niv, ivgen, NULL, sectorsize,
> >> + offset, buf, len,
> >> qcrypto_cipher_encrypt,
> >> + errp);
> >> }
> >
> > ...and get the mutex passed into these functions, as its easier to just
> > know the ivgen is always protected, and not have to trace back the callpath
> > to see if the usage is safe.
>
> but there places, where these helpers called without any QCryptoBlock, when
> we just
> have local cipher and ivgen, so there is no mutex and it's not needed.
Ah, ok.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
