[PATCH v3] tty: rocket: Fix a kernel address leakage in rp_ioctl

From: Fuqian Huang
Date: Sun Mar 31 2019 - 08:01:03 EST


If the cmd is RCPK_GET_STRUCT, copy_to_user will copy info to
user space. As info->port.ops is the address of a constant object
rocket_port_ops (assigned in init_r_port), a kernel address leakage.

This patch sets all the pointer fields to NULL before copy the
object to user space to avoid kernel address leakage.

All pointer fields except
.magic field and the pointer in .dep_map field in struct mutex
which only exist under debug configurations of CONFIG_DEBUG_MUTEXES
and CONFIG_DEBUG_LOCK_ALLOC
are set to NULL.

The set NULL pointer fields are address of kernel objects:
- pointers to constant objects: port.ops, port.client_ops
- pointers to tty_struct instance
- pointer.xmit_buf (allocated in rp_open)
- pointer fields of wait_queue_head_t and struct mutex

I cannot think of a scenario where user space needs the address of
these kernel objects. So I set them all NULL before copy_to_user.
Another reason is that I have checked all uses of copy_to_user and
copy_from_user in this file, they do not use any of these pointer
fields. So these pointer fields can be set NULL safely.

Signed-off-by: Fuqian Huang <huangfq.daxian@xxxxxxxxx>
---
drivers/tty/rocket.c | 45 +++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 43 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/rocket.c b/drivers/tty/rocket.c
index b121d8f8f3d7..df0b8ebab266 100644
--- a/drivers/tty/rocket.c
+++ b/drivers/tty/rocket.c
@@ -1271,6 +1271,48 @@ static int get_version(struct r_port *info, struct rocket_version __user *retver
return 0;
}

+static int get_struct(struct r_port *info, void *argp)
+{
+ struct r_port *new;
+ int ret = 0;
+
+ new = kzalloc(sizeof(struct r_port), GFP_KERNEL);
+ if (!new)
+ return -ENOMEM;
+ memcpy(new, info, sizeof(struct r_port));
+ new->port.buf.head = NULL;
+ memset(&new->port.buf.work.entry, 0, sizeof(struct list_head));
+ new->port.buf.work.func = NULL;
+#ifdef CONFIG_LOCKDEP
+ new->port.buf.work.lockdep_map.key = NULL;
+ memset(&new->port.buf.work.lockdep_map.class_cache, 0,
+ sizeof(struct lock_class *) * NR_LOCKDEP_CACHING_CLASSES);
+ new->port.buf.work.lockdep_map.name = NULL;
+#endif
+ memset(&new->port.buf.lock.wait_list, 0, sizeof(struct list_head));
+ new->port.buf.sentinel.next = NULL;
+ memset(&new->port.buf.free, 0, sizeof(struct llist_head));
+ new->port.buf.tail = NULL;
+ new->port.tty = NULL;
+ new->port.itty = NULL;
+ new->port.ops = NULL;
+ new->port.client_ops = NULL;
+ memset(&new->port.open_wait.head, 0, sizeof(struct list_head));
+ memset(&new->port.delta_msr_wait.head, 0, sizeof(struct list_head));
+ memset(&new->port.mutex.wait_list, 0, sizeof(struct list_head));
+ memset(&new->port.buf_mutex.wait_list, 0, sizeof(struct list_head));
+ new->port.xmit_buf = NULL;
+ new->port.client_data = NULL;
+ new->ctlp = NULL;
+ new->channel.CtlP = NULL;
+ new->xmit_buf = NULL;
+ memset(&new->write_mtx.wait_list, 0, sizeof(struct list_head));
+ if (copy_to_user(argp, new, sizeof(struct r_port)))
+ ret = -EFAULT;
+ kfree(new);
+ return ret;
+}
+
/* IOCTL call handler into the driver */
static int rp_ioctl(struct tty_struct *tty,
unsigned int cmd, unsigned long arg)
@@ -1284,8 +1326,7 @@ static int rp_ioctl(struct tty_struct *tty,

switch (cmd) {
case RCKP_GET_STRUCT:
- if (copy_to_user(argp, info, sizeof (struct r_port)))
- ret = -EFAULT;
+ ret = get_struct(info, argp);
break;
case RCKP_GET_CONFIG:
ret = get_config(info, argp);
--
2.11.0