On Thu, Sep 19, 2013 at 04:11:57PM +0200, Dominik Paulus wrote:
> +/*
> + * Perform encryption/decryption on one chunk of data.
> + * Uses global crypto state stored in usbip_device.
> + * Parameters:
> + * encrypt: 1 to perform encryption, 0 to perform decryption operation
Make this a define:
#define USBIP_ENCRYPT 1
#define USBIP_ENCRYPT 0
> + * packetsize: Size of the encrypted packet, including the authentication tag,
> + * not including the associated data (length field).
> + * plaintext and ciphertext have to be appropiately managed by the caller
> + * (i.e. they must be at least packetsize bytes long).
> + * Returns: 0 on success
> + */
> +static int usbip_crypt(struct usbip_device *ud, int encrypt, uint32_t
> + packetsize, unsigned char *plaintext, unsigned char
> + *ciphertext)
Don't break put line breaks between the type and the variable name. It
should be:
static int usbip_crypt(struct usbip_device *ud, int encrypt,
uint32_t packetsize, unsigned char *plaintext,
unsigned char *ciphertext)
This applies to earlier patches in this series as well.
> +{
> + struct crypto_aead *tfm;
> + struct aead_request *req;
> + struct tcrypt_result result;
> + struct scatterlist plain, cipher, assoc;
> + char iv[16];
> + u64 *iv_num;
> + u64 iv_net;
> + const int plainsize = packetsize - USBIP_AUTHSIZE;
Is it possible that packetsize is less than USBIP_AUTHSIZE?
> + int ret;
> +
> + memset(iv, 0, sizeof(iv));
> + if (encrypt) {
> + tfm = ud->tfm_send;
> + iv_num = &ud->ctr_send;
> + } else {
> + tfm = ud->tfm_recv;
> + iv_num = &ud->ctr_recv;
> + }
> + iv_net = cpu_to_be64(*iv_num);
> + memcpy(iv, &iv_net, sizeof(iv_net));
> +
> + req = aead_request_alloc(tfm, GFP_KERNEL);
> + if (IS_ERR(req))
> + return -PTR_ERR(req);
> +
> + init_completion(&result.completion);
> + aead_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
> + tcrypt_complete, &result);
Align this up like:
aead_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
tcrypt_complete, &result);
> +
> + sg_init_one(&cipher, ciphertext, packetsize);
> + sg_init_one(&plain, plaintext, plainsize);
> + crypto_aead_clear_flags(tfm, ~0);
> +
> + if (encrypt)
> + aead_request_set_crypt(req, &plain, &cipher, plainsize, iv);
> + else
> + aead_request_set_crypt(req, &cipher, &plain, packetsize, iv);
> + packetsize = cpu_to_be32(packetsize);
> + sg_init_one(&assoc, &packetsize, sizeof(packetsize));
> + /* Associated data: Unencrypted length tag */
> + aead_request_set_assoc(req, &assoc, sizeof(packetsize));
> +
> + if (encrypt)
> + ret = crypto_aead_encrypt(req);
> + else
> + ret = crypto_aead_decrypt(req);
> +
Good on you for figuring out what crypto_aead_en/decrypt() returns.
Where are these functions documented?
> + switch (ret) {
> + case 0: /* Success */
> + break;
> + case -EINPROGRESS:
> + case -EBUSY:
> + wait_for_completion(&result.completion);
> + break;
> + default:
> + aead_request_free(req);
> + return ret;
> + }
> +
regards,
dan carpenter
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
