[S390] hypfs: Move buffer allocation from open to read

Currently the buffer for diagnose data is allocated in the open function
of the debugfs file and is released in the close function. This has the
drawback that a user (root) can pin that memory by not closing the file.
This patch moves the buffer allocation to the read function. The buffer is
automatically released after the buffer is copied to userspace.

Signed-off-by: Michael Holzheu <holzheu@linux.vnet.ibm.com>
Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>

arch/s390/hypfs/Makefile

@@ -4,4 +4,4 @@
 
 obj-$(CONFIG_S390_HYPFS_FS) += s390_hypfs.o
 
-s390_hypfs-objs := inode.o hypfs_diag.o hypfs_vm.o
+s390_hypfs-objs := inode.o hypfs_diag.o hypfs_vm.o hypfs_dbfs.o

arch/s390/hypfs/hypfs.h

@@ -12,6 +12,8 @@
 #include <linux/fs.h>
 #include <linux/types.h>
 #include <linux/debugfs.h>
+#include <linux/workqueue.h>
+#include <linux/kref.h>
 
 #define REG_FILE_MODE    0440
 #define UPDATE_FILE_MODE 0220
@@ -38,6 +40,33 @@ extern int hypfs_vm_init(void);
 extern void hypfs_vm_exit(void);
 extern int hypfs_vm_create_files(struct super_block *sb, struct dentry *root);
 
-/* Directory for debugfs files */
+/* debugfs interface */
-extern struct dentry *hypfs_dbfs_dir;
+struct hypfs_dbfs_file;
+
+struct hypfs_dbfs_data {
+	void			*buf;
+	void			*buf_free_ptr;
+	size_t			size;
+	struct hypfs_dbfs_file	*dbfs_file;;
+	struct kref		kref;
+};
+
+struct hypfs_dbfs_file {
+	const char	*name;
+	int		(*data_create)(void **data, void **data_free_ptr,
+				       size_t *size);
+	void		(*data_free)(const void *buf_free_ptr);
+
+	/* Private data for hypfs_dbfs.c */
+	struct hypfs_dbfs_data	*data;
+	struct delayed_work	data_free_work;
+	struct mutex		lock;
+	struct dentry		*dentry;
+};
+
+extern int hypfs_dbfs_init(void);
+extern void hypfs_dbfs_exit(void);
+extern int hypfs_dbfs_create_file(struct hypfs_dbfs_file *df);
+extern void hypfs_dbfs_remove_file(struct hypfs_dbfs_file *df);
+
 #endif /* _HYPFS_H_ */

arch/s390/hypfs/hypfs_dbfs.c

@@ -0,0 +1,116 @@
+/*
+ * Hypervisor filesystem for Linux on s390 - debugfs interface
+ *
+ * Copyright (C) IBM Corp. 2010
+ * Author(s): Michael Holzheu <holzheu@linux.vnet.ibm.com>
+ */
+
+#include <linux/slab.h>
+#include "hypfs.h"
+
+static struct dentry *dbfs_dir;
+
+static struct hypfs_dbfs_data *hypfs_dbfs_data_alloc(struct hypfs_dbfs_file *f)
+{
+	struct hypfs_dbfs_data *data;
+
+	data = kmalloc(sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return NULL;
+	kref_init(&data->kref);
+	data->dbfs_file = f;
+	return data;
+}
+
+static void hypfs_dbfs_data_free(struct kref *kref)
+{
+	struct hypfs_dbfs_data *data;
+
+	data = container_of(kref, struct hypfs_dbfs_data, kref);
+	data->dbfs_file->data_free(data->buf_free_ptr);
+	kfree(data);
+}
+
+static void data_free_delayed(struct work_struct *work)
+{
+	struct hypfs_dbfs_data *data;
+	struct hypfs_dbfs_file *df;
+
+	df = container_of(work, struct hypfs_dbfs_file, data_free_work.work);
+	mutex_lock(&df->lock);
+	data = df->data;
+	df->data = NULL;
+	mutex_unlock(&df->lock);
+	kref_put(&data->kref, hypfs_dbfs_data_free);
+}
+
+static ssize_t dbfs_read(struct file *file, char __user *buf,
+			 size_t size, loff_t *ppos)
+{
+	struct hypfs_dbfs_data *data;
+	struct hypfs_dbfs_file *df;
+	ssize_t rc;
+
+	if (*ppos != 0)
+		return 0;
+
+	df = file->f_path.dentry->d_inode->i_private;
+	mutex_lock(&df->lock);
+	if (!df->data) {
+		data = hypfs_dbfs_data_alloc(df);
+		if (!data) {
+			mutex_unlock(&df->lock);
+			return -ENOMEM;
+		}
+		rc = df->data_create(&data->buf, &data->buf_free_ptr,
+				     &data->size);
+		if (rc) {
+			mutex_unlock(&df->lock);
+			kfree(data);
+			return rc;
+		}
+		df->data = data;
+		schedule_delayed_work(&df->data_free_work, HZ);
+	}
+	data = df->data;
+	kref_get(&data->kref);
+	mutex_unlock(&df->lock);
+
+	rc = simple_read_from_buffer(buf, size, ppos, data->buf, data->size);
+	kref_put(&data->kref, hypfs_dbfs_data_free);
+	return rc;
+}
+
+static const struct file_operations dbfs_ops = {
+	.read		= dbfs_read,
+	.llseek		= no_llseek,
+};
+
+int hypfs_dbfs_create_file(struct hypfs_dbfs_file *df)
+{
+	df->dentry = debugfs_create_file(df->name, 0400, dbfs_dir, df,
+					 &dbfs_ops);
+	if (IS_ERR(df->dentry))
+		return PTR_ERR(df->dentry);
+	mutex_init(&df->lock);
+	INIT_DELAYED_WORK(&df->data_free_work, data_free_delayed);
+	return 0;
+}
+
+void hypfs_dbfs_remove_file(struct hypfs_dbfs_file *df)
+{
+	debugfs_remove(df->dentry);
+}
+
+int hypfs_dbfs_init(void)
+{
+	dbfs_dir = debugfs_create_dir("s390_hypfs", NULL);
+	if (IS_ERR(dbfs_dir))
+		return PTR_ERR(dbfs_dir);
+	return 0;
+}
+
+void hypfs_dbfs_exit(void)
+{
+	debugfs_remove(dbfs_dir);
+}

arch/s390/hypfs/hypfs_diag.c

@@ -555,81 +555,38 @@ struct dbfs_d204 {
 	char			buf[];	/* d204 buffer */
 } __attribute__ ((packed));
 
-struct dbfs_d204_private {
+static int dbfs_d204_create(void **data, void **data_free_ptr, size_t *size)
-	struct dbfs_d204	*d204;	/* Aligned d204 data with header */
-	void			*base;	/* Base pointer (needed for vfree) */
-};
-
-static int dbfs_d204_open(struct inode *inode, struct file *file)
 {
-	struct dbfs_d204_private *data;
 	struct dbfs_d204 *d204;
 	int rc, buf_size;
+	void *base;
 
-	data = kzalloc(sizeof(*data), GFP_KERNEL);
-	if (!data)
-		return -ENOMEM;
 	buf_size = PAGE_SIZE * (diag204_buf_pages + 1) + sizeof(d204->hdr);
-	data->base = vmalloc(buf_size);
+	base = vmalloc(buf_size);
-	if (!data->base) {
+	if (!base)
-		rc = -ENOMEM;
+		return -ENOMEM;
-		goto fail_kfree_data;
+	memset(base, 0, buf_size);
+	d204 = page_align_ptr(base + sizeof(d204->hdr)) - sizeof(d204->hdr);
+	rc = diag204_do_store(d204->buf, diag204_buf_pages);
+	if (rc) {
+		vfree(base);
+		return rc;
 	}
-	memset(data->base, 0, buf_size);
-	d204 = page_align_ptr(data->base + sizeof(d204->hdr))
-		- sizeof(d204->hdr);
-	rc = diag204_do_store(&d204->buf, diag204_buf_pages);
-	if (rc)
-		goto fail_vfree_base;
 	d204->hdr.version = DBFS_D204_HDR_VERSION;
 	d204->hdr.len = PAGE_SIZE * diag204_buf_pages;
 	d204->hdr.sc = diag204_store_sc;
-	data->d204 = d204;
+	*data = d204;
-	file->private_data = data;
+	*data_free_ptr = base;
-	return nonseekable_open(inode, file);
+	*size = d204->hdr.len + sizeof(struct dbfs_d204_hdr);
-
-fail_vfree_base:
-	vfree(data->base);
-fail_kfree_data:
-	kfree(data);
-	return rc;
-}
-
-static int dbfs_d204_release(struct inode *inode, struct file *file)
-{
-	struct dbfs_d204_private *data = file->private_data;
-
-	vfree(data->base);
-	kfree(data);
 	return 0;
 }
 
-static ssize_t dbfs_d204_read(struct file *file, char __user *buf,
+static struct hypfs_dbfs_file dbfs_file_d204 = {
-			      size_t size, loff_t *ppos)
+	.name		= "diag_204",
-{
+	.data_create	= dbfs_d204_create,
-	struct dbfs_d204_private *data = file->private_data;
+	.data_free	= vfree,
-
-	return simple_read_from_buffer(buf, size, ppos, data->d204,
-				       data->d204->hdr.len +
-				       sizeof(data->d204->hdr));
-}
-
-static const struct file_operations dbfs_d204_ops = {
-	.open		= dbfs_d204_open,
-	.read		= dbfs_d204_read,
-	.release	= dbfs_d204_release,
-	.llseek		= no_llseek,
 };
 
-static int hypfs_dbfs_init(void)
-{
-	dbfs_d204_file = debugfs_create_file("diag_204", 0400, hypfs_dbfs_dir,
-					     NULL, &dbfs_d204_ops);
-	if (IS_ERR(dbfs_d204_file))
-		return PTR_ERR(dbfs_d204_file);
-	return 0;
-}
-
 __init int hypfs_diag_init(void)
 {
 	int rc;
@@ -639,7 +596,7 @@ __init int hypfs_diag_init(void)
 		return -ENODATA;
 	}
 	if (diag204_info_type == INFO_EXT) {
-		rc = hypfs_dbfs_init();
+		rc = hypfs_dbfs_create_file(&dbfs_file_d204);
 		if (rc)
 			return rc;
 	}
@@ -660,6 +617,7 @@ void hypfs_diag_exit(void)
 	debugfs_remove(dbfs_d204_file);
 	diag224_delete_name_table();
 	diag204_free_buffer();
+	hypfs_dbfs_remove_file(&dbfs_file_d204);
 }
 
 /*

arch/s390/hypfs/hypfs_vm.c

@@ -20,8 +20,6 @@ static char local_guest[] = "        ";
 static char all_guests[] = "*       ";
 static char *guest_query;
 
-static struct dentry *dbfs_d2fc_file;
-
 struct diag2fc_data {
 	__u32 version;
 	__u32 flags;
@@ -104,7 +102,7 @@ static void *diag2fc_store(char *query, unsigned int *count, int offset)
 	return data;
 }
 
-static void diag2fc_free(void *data)
+static void diag2fc_free(const void *data)
 {
 	vfree(data);
 }
@@ -239,43 +237,29 @@ struct dbfs_d2fc {
 	char			buf[];	/* d2fc buffer */
 } __attribute__ ((packed));
 
-static int dbfs_d2fc_open(struct inode *inode, struct file *file)
+static int dbfs_diag2fc_create(void **data, void **data_free_ptr, size_t *size)
 {
-	struct dbfs_d2fc *data;
+	struct dbfs_d2fc *d2fc;
 	unsigned int count;
 
-	data = diag2fc_store(guest_query, &count, sizeof(data->hdr));
+	d2fc = diag2fc_store(guest_query, &count, sizeof(d2fc->hdr));
-	if (IS_ERR(data))
+	if (IS_ERR(d2fc))
-		return PTR_ERR(data);
+		return PTR_ERR(d2fc);
-	get_clock_ext(data->hdr.tod_ext);
+	get_clock_ext(d2fc->hdr.tod_ext);
-	data->hdr.len = count * sizeof(struct diag2fc_data);
+	d2fc->hdr.len = count * sizeof(struct diag2fc_data);
-	data->hdr.version = DBFS_D2FC_HDR_VERSION;
+	d2fc->hdr.version = DBFS_D2FC_HDR_VERSION;
-	data->hdr.count = count;
+	d2fc->hdr.count = count;
-	memset(&data->hdr.reserved, 0, sizeof(data->hdr.reserved));
+	memset(&d2fc->hdr.reserved, 0, sizeof(d2fc->hdr.reserved));
-	file->private_data = data;
+	*data = d2fc;
-	return nonseekable_open(inode, file);
+	*data_free_ptr = d2fc;
-}
+	*size = d2fc->hdr.len + sizeof(struct dbfs_d2fc_hdr);
-
-static int dbfs_d2fc_release(struct inode *inode, struct file *file)
-{
-	diag2fc_free(file->private_data);
 	return 0;
 }
 
-static ssize_t dbfs_d2fc_read(struct file *file, char __user *buf,
+static struct hypfs_dbfs_file dbfs_file_2fc = {
-				    size_t size, loff_t *ppos)
+	.name		= "diag_2fc",
-{
+	.data_create	= dbfs_diag2fc_create,
-	struct dbfs_d2fc *data = file->private_data;
+	.data_free	= diag2fc_free,
-
-	return simple_read_from_buffer(buf, size, ppos, data, data->hdr.len +
-				       sizeof(struct dbfs_d2fc_hdr));
-}
-
-static const struct file_operations dbfs_d2fc_ops = {
-	.open		= dbfs_d2fc_open,
-	.read		= dbfs_d2fc_read,
-	.release	= dbfs_d2fc_release,
-	.llseek		= no_llseek,
 };
 
 int hypfs_vm_init(void)
@@ -288,18 +272,12 @@ int hypfs_vm_init(void)
 		guest_query = local_guest;
 	else
 		return -EACCES;
-
+	return hypfs_dbfs_create_file(&dbfs_file_2fc);
-	dbfs_d2fc_file = debugfs_create_file("diag_2fc", 0400, hypfs_dbfs_dir,
-					     NULL, &dbfs_d2fc_ops);
-	if (IS_ERR(dbfs_d2fc_file))
-		return PTR_ERR(dbfs_d2fc_file);
-
-	return 0;
 }
 
 void hypfs_vm_exit(void)
 {
 	if (!MACHINE_IS_VM)
 		return;
-	debugfs_remove(dbfs_d2fc_file);
+	hypfs_dbfs_remove_file(&dbfs_file_2fc);
 }

arch/s390/hypfs/inode.c

@@ -46,8 +46,6 @@ static const struct super_operations hypfs_s_ops;
 /* start of list of all dentries, which have to be deleted on update */
 static struct dentry *hypfs_last_dentry;
 
-struct dentry *hypfs_dbfs_dir;
-
 static void hypfs_update_update(struct super_block *sb)
 {
 	struct hypfs_sb_info *sb_info = sb->s_fs_info;
@@ -471,13 +469,12 @@ static int __init hypfs_init(void)
 {
 	int rc;
 
-	hypfs_dbfs_dir = debugfs_create_dir("s390_hypfs", NULL);
+	rc = hypfs_dbfs_init();
-	if (IS_ERR(hypfs_dbfs_dir))
+	if (rc)
-		return PTR_ERR(hypfs_dbfs_dir);
+		return rc;
-
 	if (hypfs_diag_init()) {
 		rc = -ENODATA;
-		goto fail_debugfs_remove;
+		goto fail_dbfs_exit;
 	}
 	if (hypfs_vm_init()) {
 		rc = -ENODATA;
@@ -499,9 +496,8 @@ fail_hypfs_vm_exit:
 	hypfs_vm_exit();
 fail_hypfs_diag_exit:
 	hypfs_diag_exit();
-fail_debugfs_remove:
+fail_dbfs_exit:
-	debugfs_remove(hypfs_dbfs_dir);
+	hypfs_dbfs_exit();
-
 	pr_err("Initialization of hypfs failed with rc=%i\n", rc);
 	return rc;
 }
@@ -510,7 +506,7 @@ static void __exit hypfs_exit(void)
 {
 	hypfs_diag_exit();
 	hypfs_vm_exit();
-	debugfs_remove(hypfs_dbfs_dir);
+	hypfs_dbfs_exit();
 	unregister_filesystem(&hypfs_type);
 	kobject_put(s390_kobj);
 }