On Thu, Sep 19, 2013 at 04:11:57PM +0200, Dominik Paulus wrote:
> +int usbip_init_crypto(struct usbip_device *ud, unsigned char *sendkey, unsigned
> + char *recvkey)
> +{
> + int ret;
> +
> + ud->use_crypto = 1;
> +
> + ud->tfm_recv = crypto_alloc_aead("gcm(aes)", 0, 0);
> + if (IS_ERR(ud->tfm_recv))
> + return -PTR_ERR(ud->tfm_recv);
PTR_ERR() already returns a negative.
> + ud->tfm_send = crypto_alloc_aead("gcm(aes)", 0, 0);
> + if (IS_ERR(ud->tfm_send)) {
> + crypto_free_aead(ud->tfm_recv);
> + return -PTR_ERR(ud->tfm_send);
Again. All the uses of PTR_ERR() in this patch have the same problem.
> + plainbuf = kmalloc(USBIP_PACKETSIZE, GFP_KERNEL);
> + if (IS_ERR(plainbuf))
> + return -PTR_ERR(plainbuf);
kmalloc() returns NULL on error and not an ERR_PTR. All the calls to
kmalloc() have this problem.
> + cipherbuf = kmalloc(USBIP_PACKETSIZE, GFP_KERNEL);
> + if (IS_ERR(cipherbuf)) {
> + kfree(plainbuf);
> + return -PTR_ERR(cipherbuf);
> + }
> +
> + while (total < size) {
> + uint32_t packetsize;
> + struct kvec recvvec;
> +
> + /*
> + * We use a global kfifo to buffer unrequested plaintext bytes.
> + * Flush this buffer first before receiving new data.
> + */
> + if (kfifo_len(&ud->recv_queue)) {
> + size_t next = min_t(size_t, kfifo_len(&ud->recv_queue),
> + size - total);
> + /* No error checking necessary - see previous line */
> + ret = kfifo_out(&ud->recv_queue, ((char *)
> + vec[0].iov_base)+total, next);
The comment assume there is only one reader and one writer at a time,
yes? The casting is not needed:
ret = kfifo_out(&ud->recv_queue,
vec[0].iov_base + total, next);
v> + total += next;
> + continue;
> + }
> +
> + /* See usbip_sendmsg() for the format of one encrypted packet */
> +
> + /*
> + * Receive size of next crypto packet
> + */
> + recvvec.iov_base = &packetsize;
> + recvvec.iov_len = sizeof(packetsize);
> +
> + ret = kernel_recvmsg(ud->tcp_socket, msg, &recvvec, 1,
> + sizeof(packetsize), flags);
> + packetsize = be32_to_cpu(packetsize);
> + if (ret <= 0) {
I think a return of zero should mean total = -EBADMSG;. In other words
this check should be "if (ret < 0) {" and we hit the next else if.
Same below again.
> + total = ret;
> + goto err;
> + } else if (ret != sizeof(packetsize)) {
> + total = -EBADMSG;
> + goto err;
> + }
> +
> + if (packetsize > USBIP_PACKETSIZE) {
> + total = -EBADMSG;
> + goto err;
> + }
> +
regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
