From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:34832)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <famz@redhat.com>) id 1aMA20-0004pl-2l
	for qemu-devel@nongnu.org; Thu, 21 Jan 2016 02:51:48 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <famz@redhat.com>) id 1aMA1z-0003bU-5u
	for qemu-devel@nongnu.org; Thu, 21 Jan 2016 02:51:48 -0500
Date: Thu, 21 Jan 2016 15:51:37 +0800
From: Fam Zheng <famz@redhat.com>
Message-ID: <20160121075137.GF31960@ad.usersys.redhat.com>
References: <1453311539-1193-1-git-send-email-berrange@redhat.com>
	<1453311539-1193-5-git-send-email-berrange@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1453311539-1193-5-git-send-email-berrange@redhat.com>
Subject: Re: [Qemu-devel] [PATCH v2 04/17] crypto: add support for
 generating initialization vectors
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: "Daniel P. Berrange" <berrange@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>, qemu-devel@nongnu.org, qemu-block@nongnu.org

On Wed, 01/20 17:38, Daniel P. Berrange wrote:
> +static int qcrypto_ivgen_essiv_init(QCryptoIVGen *ivgen,
> +                                    const uint8_t *key, size_t nkey,
> +                                    Error **errp)
> +{
> +    uint8_t *salt;
> +    size_t nhash;
> +    QCryptoIVGenESSIV *essiv = g_new0(QCryptoIVGenESSIV, 1);
> +
> +    nhash = qcrypto_hash_digest_len(ivgen->hash);
> +    /* Salt must be larger of hash size or key size */
> +    salt = g_new0(uint8_t, nhash > nkey ? nhash : nkey);
> +
> +    if (qcrypto_hash_bytes(ivgen->hash, (const gchar *)key, nkey,
> +                           &salt, &nhash,
> +                           errp) < 0) {
> +        g_free(essiv);
> +        return -1;
> +    }
> +
> +    essiv->cipher = qcrypto_cipher_new(ivgen->cipher,
> +                                       QCRYPTO_CIPHER_MODE_ECB,
> +                                       salt, nkey,
> +                                       errp);
> +    if (!essiv->cipher) {
> +        g_free(essiv);
> +        g_free(salt);
> +        return -1;
> +    }
> +
> +    g_free(salt);
> +    ivgen->private = essiv;
> +
> +    return 0;
> +}
> +
> +static int qcrypto_ivgen_essiv_calculate(QCryptoIVGen *ivgen,
> +                                         uint64_t sector,
> +                                         uint8_t *iv, size_t niv,
> +                                         Error **errp)
> +{
> +    QCryptoIVGenESSIV *essiv = ivgen->private;
> +    size_t ndata = qcrypto_cipher_get_block_len(ivgen->cipher);
> +    uint8_t *data = g_new(uint8_t, ndata);
> +
> +    sector = cpu_to_le64((uint32_t)sector);

Why casting to 32 bit?

> +    memcpy(data, (uint8_t *)&sector, ndata);
> +    if (sizeof(sector) < ndata) {
> +        memset(data + sizeof(sector), 0, ndata - sizeof(sector));
> +    }
> +
> +    if (qcrypto_cipher_encrypt(essiv->cipher,
> +                               data,
> +                               data,
> +                               ndata,
> +                               errp) < 0) {
> +        g_free(data);
> +        return -1;
> +    }
> +
> +    if (ndata > niv) {
> +        ndata = niv;
> +    }
> +    memcpy(iv, data, ndata);
> +    if (ndata < niv) {
> +        memset(iv + ndata, 0, niv - ndata);
> +    }
> +    g_free(data);
> +    return 0;
> +}
> +
> +static void qcrypto_ivgen_essiv_cleanup(QCryptoIVGen *ivgen)
> +{
> +    QCryptoIVGenESSIV *essiv = ivgen->private;
> +
> +    qcrypto_cipher_free(essiv->cipher);
> +    g_free(essiv);
> +}
> +
> +
> +struct QCryptoIVGenDriver qcrypto_ivgen_essiv = {
> +    qcrypto_ivgen_essiv_init,

It'd be more readable to explicitly write

    .init = qcrypto_ivgen_essiv_init,
    .calculate = ...,
    .cleanup = ...

but it is fine since there are only three operations now.

> +    qcrypto_ivgen_essiv_calculate,
> +    qcrypto_ivgen_essiv_cleanup,
> +};
> +