From eb2aad752c43f57e88ab9b0c3c5ee7b976ee31dd Mon Sep 17 00:00:00 2001 From: Zhen Kong Date: Mon, 31 Oct 2016 15:23:19 -0700 Subject: msm: crypto: fix issues on digest buf and copy_from_user in qcedev.c Make the digest length not larger than the size of the buffer qcedev_areq.sha_op_req.digest; and use the checked variants of the copy_from/to_user() APIs to avoid small race window of their unchecked variants. Change-Id: I3db0c20ac5fa47ed278f3d60368c406f472430c1 Signed-off-by: Zhen Kong --- drivers/crypto/msm/qcedev.c | 120 ++++++++++---------------------------------- 1 file changed, 27 insertions(+), 93 deletions(-) diff --git a/drivers/crypto/msm/qcedev.c b/drivers/crypto/msm/qcedev.c index 1402d3d..433e478 100644 --- a/drivers/crypto/msm/qcedev.c +++ b/drivers/crypto/msm/qcedev.c @@ -603,7 +603,7 @@ static int qcedev_sha_update_max_xfer(struct qcedev_async_req *qcedev_areq, while (len > 0) { user_src = (void __user *)qcedev_areq->sha_op_req.data[i].vaddr; - if (user_src && __copy_from_user(k_src, + if (user_src && copy_from_user(k_src, (void __user *)user_src, qcedev_areq->sha_op_req.data[i].len)) return -EFAULT; @@ -639,7 +639,7 @@ static int qcedev_sha_update_max_xfer(struct qcedev_async_req *qcedev_areq, /* Copy data from user src(s) */ user_src = (void __user *)qcedev_areq->sha_op_req.data[0].vaddr; - if (user_src && __copy_from_user(k_src, + if (user_src && copy_from_user(k_src, (void __user *)user_src, qcedev_areq->sha_op_req.data[0].len)) { kzfree(k_buf_src); @@ -648,7 +648,7 @@ static int qcedev_sha_update_max_xfer(struct qcedev_async_req *qcedev_areq, k_src += qcedev_areq->sha_op_req.data[0].len; for (i = 1; i < qcedev_areq->sha_op_req.entries; i++) { user_src = (void __user *)qcedev_areq->sha_op_req.data[i].vaddr; - if (user_src && __copy_from_user(k_src, + if (user_src && copy_from_user(k_src, (void __user *)user_src, qcedev_areq->sha_op_req.data[i].len)) { kzfree(k_buf_src); @@ -702,13 +702,6 @@ static int qcedev_sha_update(struct qcedev_async_req *qcedev_areq, return -EINVAL; } - /* verify address src(s) */ - for (i = 0; i < qcedev_areq->sha_op_req.entries; i++) - if (!access_ok(VERIFY_READ, - (void __user *)qcedev_areq->sha_op_req.data[i].vaddr, - qcedev_areq->sha_op_req.data[i].len)) - return -EFAULT; - if (qcedev_areq->sha_op_req.data_len > QCE_MAX_OPER_DATA) { struct qcedev_sha_op_req *saved_req; @@ -868,19 +861,7 @@ static int qcedev_hash_cmac(struct qcedev_async_req *qcedev_areq, total = qcedev_areq->sha_op_req.data_len; - /* verify address src(s) */ - for (i = 0; i < qcedev_areq->sha_op_req.entries; i++) - if (!access_ok(VERIFY_READ, - (void __user *)qcedev_areq->sha_op_req.data[i].vaddr, - qcedev_areq->sha_op_req.data[i].len)) - return -EFAULT; - - /* Verify Source Address */ - if (!access_ok(VERIFY_READ, - (void __user *)qcedev_areq->sha_op_req.authkey, - qcedev_areq->sha_op_req.authklen)) - return -EFAULT; - if (__copy_from_user(&handle->sha_ctxt.authkey[0], + if (copy_from_user(&handle->sha_ctxt.authkey[0], (void __user *)qcedev_areq->sha_op_req.authkey, qcedev_areq->sha_op_req.authklen)) return -EFAULT; @@ -900,7 +881,7 @@ static int qcedev_hash_cmac(struct qcedev_async_req *qcedev_areq, for (i = 0; i < qcedev_areq->sha_op_req.entries; i++) { user_src = (void __user *)qcedev_areq->sha_op_req.data[i].vaddr; - if (user_src && __copy_from_user(k_src, (void __user *)user_src, + if (user_src && copy_from_user(k_src, (void __user *)user_src, qcedev_areq->sha_op_req.data[i].len)) { kzfree(k_buf_src); return -EFAULT; @@ -928,12 +909,7 @@ static int qcedev_set_hmac_auth_key(struct qcedev_async_req *areq, if (areq->sha_op_req.authklen <= QCEDEV_MAX_KEY_SIZE) { qcedev_sha_init(areq, handle); - /* Verify Source Address */ - if (!access_ok(VERIFY_READ, - (void __user *)areq->sha_op_req.authkey, - areq->sha_op_req.authklen)) - return -EFAULT; - if (__copy_from_user(&handle->sha_ctxt.authkey[0], + if (copy_from_user(&handle->sha_ctxt.authkey[0], (void __user *)areq->sha_op_req.authkey, areq->sha_op_req.authklen)) return -EFAULT; @@ -1146,7 +1122,7 @@ static int qcedev_vbuf_ablk_cipher_max_xfer(struct qcedev_async_req *areq, byteoffset = areq->cipher_op_req.byteoffset; user_src = (void __user *)areq->cipher_op_req.vbuf.src[0].vaddr; - if (user_src && __copy_from_user((k_align_src + byteoffset), + if (user_src && copy_from_user((k_align_src + byteoffset), (void __user *)user_src, areq->cipher_op_req.vbuf.src[0].len)) return -EFAULT; @@ -1156,7 +1132,7 @@ static int qcedev_vbuf_ablk_cipher_max_xfer(struct qcedev_async_req *areq, for (i = 1; i < areq->cipher_op_req.entries; i++) { user_src = (void __user *)areq->cipher_op_req.vbuf.src[i].vaddr; - if (user_src && __copy_from_user(k_align_src, + if (user_src && copy_from_user(k_align_src, (void __user *)user_src, areq->cipher_op_req.vbuf.src[i].len)) { return -EFAULT; @@ -1188,7 +1164,7 @@ static int qcedev_vbuf_ablk_cipher_max_xfer(struct qcedev_async_req *areq, while (creq->data_len > 0) { if (creq->vbuf.dst[dst_i].len <= creq->data_len) { - if (err == 0 && __copy_to_user( + if (err == 0 && copy_to_user( (void __user *)creq->vbuf.dst[dst_i].vaddr, (k_align_dst + byteoffset), creq->vbuf.dst[dst_i].len)) @@ -1199,7 +1175,7 @@ static int qcedev_vbuf_ablk_cipher_max_xfer(struct qcedev_async_req *areq, creq->data_len -= creq->vbuf.dst[dst_i].len; dst_i++; } else { - if (err == 0 && __copy_to_user( + if (err == 0 && copy_to_user( (void __user *)creq->vbuf.dst[dst_i].vaddr, (k_align_dst + byteoffset), creq->data_len)) @@ -1531,36 +1507,6 @@ static int qcedev_check_cipher_params(struct qcedev_cipher_op_req *req, __func__, total, req->data_len); goto error; } - /* Verify Source Address's */ - for (i = 0, total = 0; i < req->entries; i++) { - if (total < req->data_len) { - if (!access_ok(VERIFY_READ, - (void __user *)req->vbuf.src[i].vaddr, - req->vbuf.src[i].len)) { - pr_err("%s:SRC RD_VERIFY err %d=0x%lx\n", - __func__, i, (uintptr_t) - req->vbuf.src[i].vaddr); - goto error; - } - total += req->vbuf.src[i].len; - } - } - - /* Verify Destination Address's */ - for (i = 0, total = 0; i < QCEDEV_MAX_BUFFERS; i++) { - if ((req->vbuf.dst[i].vaddr != 0) && - (total < req->data_len)) { - if (!access_ok(VERIFY_WRITE, - (void __user *)req->vbuf.dst[i].vaddr, - req->vbuf.dst[i].len)) { - pr_err("%s:DST WR_VERIFY err %d=0x%lx\n", - __func__, i, (uintptr_t) - req->vbuf.dst[i].vaddr); - goto error; - } - total += req->vbuf.dst[i].len; - } - } return 0; error: return -EINVAL; @@ -1656,11 +1602,7 @@ long qcedev_ioctl(struct file *file, unsigned cmd, unsigned long arg) switch (cmd) { case QCEDEV_IOCTL_ENC_REQ: case QCEDEV_IOCTL_DEC_REQ: - if (!access_ok(VERIFY_WRITE, (void __user *)arg, - sizeof(struct qcedev_cipher_op_req))) - return -EFAULT; - - if (__copy_from_user(&qcedev_areq.cipher_op_req, + if (copy_from_user(&qcedev_areq.cipher_op_req, (void __user *)arg, sizeof(struct qcedev_cipher_op_req))) return -EFAULT; @@ -1673,20 +1615,17 @@ long qcedev_ioctl(struct file *file, unsigned cmd, unsigned long arg) err = qcedev_vbuf_ablk_cipher(&qcedev_areq, handle); if (err) return err; - if (__copy_to_user((void __user *)arg, + if (copy_to_user((void __user *)arg, &qcedev_areq.cipher_op_req, sizeof(struct qcedev_cipher_op_req))) - return -EFAULT; + return -EFAULT; break; case QCEDEV_IOCTL_SHA_INIT_REQ: { struct scatterlist sg_src; - if (!access_ok(VERIFY_WRITE, (void __user *)arg, - sizeof(struct qcedev_sha_op_req))) - return -EFAULT; - if (__copy_from_user(&qcedev_areq.sha_op_req, + if (copy_from_user(&qcedev_areq.sha_op_req, (void __user *)arg, sizeof(struct qcedev_sha_op_req))) return -EFAULT; @@ -1696,9 +1635,9 @@ long qcedev_ioctl(struct file *file, unsigned cmd, unsigned long arg) err = qcedev_hash_init(&qcedev_areq, handle, &sg_src); if (err) return err; - if (__copy_to_user((void __user *)arg, &qcedev_areq.sha_op_req, + if (copy_to_user((void __user *)arg, &qcedev_areq.sha_op_req, sizeof(struct qcedev_sha_op_req))) - return -EFAULT; + return -EFAULT; } handle->sha_ctxt.init_done = true; break; @@ -1708,11 +1647,8 @@ long qcedev_ioctl(struct file *file, unsigned cmd, unsigned long arg) case QCEDEV_IOCTL_SHA_UPDATE_REQ: { struct scatterlist sg_src; - if (!access_ok(VERIFY_WRITE, (void __user *)arg, - sizeof(struct qcedev_sha_op_req))) - return -EFAULT; - if (__copy_from_user(&qcedev_areq.sha_op_req, + if (copy_from_user(&qcedev_areq.sha_op_req, (void __user *)arg, sizeof(struct qcedev_sha_op_req))) return -EFAULT; @@ -1734,10 +1670,15 @@ long qcedev_ioctl(struct file *file, unsigned cmd, unsigned long arg) return err; } + if (handle->sha_ctxt.diglen > QCEDEV_MAX_SHA_DIGEST) { + pr_err("Invalid sha_ctxt.diglen %d\n", + handle->sha_ctxt.diglen); + return -EINVAL; + } memcpy(&qcedev_areq.sha_op_req.digest[0], &handle->sha_ctxt.digest[0], handle->sha_ctxt.diglen); - if (__copy_to_user((void __user *)arg, &qcedev_areq.sha_op_req, + if (copy_to_user((void __user *)arg, &qcedev_areq.sha_op_req, sizeof(struct qcedev_sha_op_req))) return -EFAULT; } @@ -1749,11 +1690,7 @@ long qcedev_ioctl(struct file *file, unsigned cmd, unsigned long arg) pr_err("%s Init was not called\n", __func__); return -EINVAL; } - if (!access_ok(VERIFY_WRITE, (void __user *)arg, - sizeof(struct qcedev_sha_op_req))) - return -EFAULT; - - if (__copy_from_user(&qcedev_areq.sha_op_req, + if (copy_from_user(&qcedev_areq.sha_op_req, (void __user *)arg, sizeof(struct qcedev_sha_op_req))) return -EFAULT; @@ -1767,7 +1704,7 @@ long qcedev_ioctl(struct file *file, unsigned cmd, unsigned long arg) memcpy(&qcedev_areq.sha_op_req.digest[0], &handle->sha_ctxt.digest[0], handle->sha_ctxt.diglen); - if (__copy_to_user((void __user *)arg, &qcedev_areq.sha_op_req, + if (copy_to_user((void __user *)arg, &qcedev_areq.sha_op_req, sizeof(struct qcedev_sha_op_req))) return -EFAULT; handle->sha_ctxt.init_done = false; @@ -1776,11 +1713,8 @@ long qcedev_ioctl(struct file *file, unsigned cmd, unsigned long arg) case QCEDEV_IOCTL_GET_SHA_REQ: { struct scatterlist sg_src; - if (!access_ok(VERIFY_WRITE, (void __user *)arg, - sizeof(struct qcedev_sha_op_req))) - return -EFAULT; - if (__copy_from_user(&qcedev_areq.sha_op_req, + if (copy_from_user(&qcedev_areq.sha_op_req, (void __user *)arg, sizeof(struct qcedev_sha_op_req))) return -EFAULT; @@ -1798,7 +1732,7 @@ long qcedev_ioctl(struct file *file, unsigned cmd, unsigned long arg) memcpy(&qcedev_areq.sha_op_req.digest[0], &handle->sha_ctxt.digest[0], handle->sha_ctxt.diglen); - if (__copy_to_user((void __user *)arg, &qcedev_areq.sha_op_req, + if (copy_to_user((void __user *)arg, &qcedev_areq.sha_op_req, sizeof(struct qcedev_sha_op_req))) return -EFAULT; } -- cgit v1.1