linux-kernel - [PATCH v3] tty: rocket: Fix a kernel address leakage in rp

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20190331115127.29731-1-huangfq.daxian@gmail.com>
Date:   Sun, 31 Mar 2019 19:51:27 +0800
From:   Fuqian Huang <huangfq.daxian@...il.com>
To:     unlisted-recipients:; (no To-header on input)
Cc:     Fuqian Huang <huangfq.daxian@...il.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jiri Slaby <jslaby@...e.com>, linux-kernel@...r.kernel.org
Subject: [PATCH v3] tty: rocket: Fix a kernel address leakage in rp_ioctl

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@...il.com>
---
 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