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-prev] [thread-next>] [day] [month] [year] [list]
Date:	Tue,  7 Apr 2015 10:18:18 +0200
From:	Tom Van Braeckel <tomvanbraeckel@...il.com>
To:	rusty@...tcorp.com.au, lguest@...ts.ozlabs.org
Cc:	linux-kernel@...r.kernel.org, fengguang.wu@...el.com,
	gregkh@...uxfoundation.org, lkp@...org,
	Tom Van Braeckel <tomvanbraeckel@...il.com>
Subject: [PATCH] lguest: explicitly setup /dev/lguest private_data

The private_data member of the /dev/lguest device file is used to hold
the current struct lguest and needs to be set to NULL to signify that
no initialization has taken place.

We explicitly set it to NULL to be independent of whatever value the
misc subsystem initializes it to.

Signed-off-by: Tom Van Braeckel <tomvanbraeckel@...il.com>
---
Backstory:
==========
The misc subsystem used to initialize a file's private_data to point to
the misc device when a driver had registered a custom open file
operation and initialized it to NULL when a custom open file operation
had *not* been provided.

This subtle quirk was confusing, to the point where kernel code
registered *empty* file open operations to have private_data point to
the misc device structure.

And it lead to bugs, where the addition or removal of a custom open
file operation surprisingly changed the initial contents of a file's
private_data structure.

The misc subsystem is currently underdoing changes to *always* set
private_data to point to the misc device instead of only doing this
when a custom open file operation has been registered.

Intel's 0day kernel testing robot discovered that the lguest driver
depended on it implicitly being initialized to NULL, as Fengguang Wu
reported. Thanks a lot for all the hard work!

 drivers/lguest/lguest_user.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/lguest/lguest_user.c b/drivers/lguest/lguest_user.c
index c4c6113..054bf70 100644
--- a/drivers/lguest/lguest_user.c
+++ b/drivers/lguest/lguest_user.c
@@ -98,6 +98,17 @@ static int trap(struct lg_cpu *cpu, const unsigned long __user *input)
 	return 0;
 }
 
+/*
+ * Set up the /dev/lguest file structure
+ * The file's private_data will hold the "struct lguest" after
+ * initialization and is used to check whether it is already initialized.
+ */
+static int open(struct inode *inode, struct file *file)
+{
+	file->private_data = NULL;
+	return 0;
+}
+
 /*L:040
  * Once our Guest is initialized, the Launcher makes it run by reading
  * from /dev/lguest.
@@ -405,10 +416,11 @@ static int close(struct inode *inode, struct file *file)
  *
  * We begin our understanding with the Host kernel interface which the Launcher
  * uses: reading and writing a character device called /dev/lguest.  All the
- * work happens in the read(), write() and close() routines:
+ * work happens in the open(), read(), write() and close() routines:
  */
 static const struct file_operations lguest_fops = {
 	.owner	 = THIS_MODULE,
+	.open	 = open,
 	.release = close,
 	.write	 = write,
 	.read	 = read,
-- 
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ