On 11/12/2014 08:05 AM, Stephan Mueller wrote:
This patch adds the random number generator support for AF_ALG. A random number generator's purpose is to generate data without requiring the caller to provide any data. Therefore, the AF_ALG interface handler for RNGs only implements a callback handler for recvmsg.
...
+static int rng_recvmsg(struct kiocb *unused, struct socket *sock, + struct msghdr *msg, size_t len, int flags) +{ + struct sock *sk = sock->sk; + struct alg_sock *ask = alg_sk(sk); + struct rng_ctx *ctx = ask->private; + int err = -EFAULT; + + if (0 == len)
if (len == 0) ... [And also other places.] We don't use Yoda condition style in the kernel.
+ return 0; + if (MAXSIZE < len) + len = MAXSIZE; + + lock_sock(sk); + len = crypto_rng_get_bytes(ctx->drng, ctx->result, len); + if (0 > len) + goto unlock; + + err = memcpy_toiovec(msg->msg_iov, ctx->result, len); + memset(ctx->result, 0, err); +
This looks buggy. If copy_to_user() fails from within memcpy_toiovec(), we call memset() with a negative return value which is interpreted as size_t and thus causes a buffer overflow writing beyond ctx->result, no? If it succeeds, we call memset(ctx->result, 0, 0) .....
+unlock: + release_sock(sk); + + return err ? err : len; +} + +static struct proto_ops algif_rng_ops = { + .family = PF_ALG, + + .connect = sock_no_connect, + .socketpair = sock_no_socketpair, + .getname = sock_no_getname, + .ioctl = sock_no_ioctl, + .listen = sock_no_listen, + .shutdown = sock_no_shutdown, + .getsockopt = sock_no_getsockopt, + .mmap = sock_no_mmap, + .bind = sock_no_bind, + .accept = sock_no_accept, + .setsockopt = sock_no_setsockopt, + .poll = sock_no_poll, + .sendmsg = sock_no_sendmsg, + .sendpage = sock_no_sendpage, + + .release = af_alg_release, + .recvmsg = rng_recvmsg, +}; + +static void *rng_bind(const char *name, u32 type, u32 mask) +{ + return crypto_alloc_rng(name, type, mask); +} + +static void rng_release(void *private) +{ + crypto_free_rng(private); +} + +static void rng_sock_destruct(struct sock *sk) +{ + struct alg_sock *ask = alg_sk(sk); + struct rng_ctx *ctx = ask->private; + + memset(ctx->result, 0, MAXSIZE);
memset(ctx->result, 0, sizeof(ctx->result));
+ sock_kfree_s(sk, ctx, ctx->len); + af_alg_release_parent(sk); +} + +static int rng_accept_parent(void *private, struct sock *sk) +{ + struct rng_ctx *ctx; + struct alg_sock *ask = alg_sk(sk); + unsigned int len = sizeof(*ctx); + int seedsize = crypto_rng_seedsize(private); + int ret = -ENOMEM; + + ctx = sock_kmalloc(sk, len, GFP_KERNEL); + if (!ctx) + return -ENOMEM; + memset(ctx->result, 0, MAXSIZE);
Ditto...
+ ctx->len = len; + + if (seedsize) { + u8 *buf = kmalloc(seedsize, GFP_KERNEL); + if (!buf) + goto err; + get_random_bytes(buf, seedsize); + ret = crypto_rng_reset(private, buf, len); + kzfree(buf);
-- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html