Message-Id: <1347505915-27984-1-git-send-email-anatol.pomozov@gmail.com>
Date:	Wed, 12 Sep 2012 20:11:55 -0700
From:	Anatol Pomozov <anatol.pomozov@...il.com>
To:	linux-fsdevel@...r.kernel.org, viro@...iv.linux.org.uk,
	linux-kernel@...r.kernel.org
Cc:	tytso@....edu, akpm@...ux-foundation.org,
	Anatol Pomozov <anatol.pomozov@...il.com>
Subject: [PATCH] fs: Preserve error code in get_empty_filp()

Allocating a file structure in function get_empty_filp() might fail because
of several reasons:
 - not enough memory for file structures
 - operation is not allowed
 - user is over its limit

Currently the function returns NULL in all cases and we loose the exact
reason of the error. All callers of get_empty_filp() assume that the function
can fail with ENFILE only.

Return error through pointer. Change all callers to preserve this error code.

Signed-off-by: Anatol Pomozov <anatol.pomozov@...il.com>
Reviewed-by: "Theodore Ts'o" <tytso@....edu>
---
 arch/ia64/kernel/perfmon.c |  4 ++--
 fs/anon_inodes.c           |  5 +++--
 fs/file_table.c            | 22 ++++++++++++++--------
 fs/hugetlbfs/inode.c       |  5 +++--
 fs/namei.c                 |  4 ++--
 fs/open.c                  |  5 ++---
 fs/pipe.c                  |  9 ++++++---
 ipc/shm.c                  |  4 +++-
 mm/shmem.c                 |  5 +++--
 net/socket.c               |  4 ++--
 10 files changed, 40 insertions(+), 27 deletions(-)

diff --git a/arch/ia64/kernel/perfmon.c b/arch/ia64/kernel/perfmon.c
index 3fa4bc5..da2dcce 100644
--- a/arch/ia64/kernel/perfmon.c
+++ b/arch/ia64/kernel/perfmon.c
@@ -2221,9 +2221,9 @@ pfm_alloc_file(pfm_context_t *ctx)
 	d_add(path.dentry, inode);
 
 	file = alloc_file(&path, FMODE_READ, &pfm_file_ops);
-	if (!file) {
+	if (IS_ERR(file)) {
 		path_put(&path);
-		return ERR_PTR(-ENFILE);
+		return file;
 	}
 
 	file->f_flags = O_RDONLY;
diff --git a/fs/anon_inodes.c b/fs/anon_inodes.c
index 28d39fb..73536e3 100644
--- a/fs/anon_inodes.c
+++ b/fs/anon_inodes.c
@@ -160,10 +160,11 @@ struct file *anon_inode_getfile(const char *name,
 
 	d_instantiate(path.dentry, anon_inode_inode);
 
-	error = -ENFILE;
 	file = alloc_file(&path, OPEN_FMODE(flags), fops);
-	if (!file)
+	if (IS_ERR(file)) {
+		error = PTR_ERR(file);
 		goto err_dput;
+	}
 	file->f_mapping = anon_inode_inode->i_mapping;
 
 	file->f_pos = 0;
diff --git a/fs/file_table.c b/fs/file_table.c
index 701985e..3b3f4d7 100644
--- a/fs/file_table.c
+++ b/fs/file_table.c
@@ -94,8 +94,8 @@ int proc_nr_files(ctl_table *table, int write,
 #endif
 
 /* Find an unused file structure and return a pointer to it.
- * Returns NULL, if there are no more free file structures or
- * we run out of memory.
+ * Returns an error pointer if some error happend e.g. we over file
+ * structures limit, run out of memory or operation is not permitted.
  *
  * Be very careful using this.  You are responsible for
  * getting write access to any mount that you might assign
@@ -108,6 +108,7 @@ struct file *get_empty_filp(void)
 	const struct cred *cred = current_cred();
 	static long old_max;
 	struct file * f;
+	int error;
 
 	/*
 	 * Privileged users can go above max_files
@@ -117,17 +118,22 @@ struct file *get_empty_filp(void)
 		 * percpu_counters are inaccurate.  Do an expensive check before
 		 * we go and fail.
 		 */
-		if (percpu_counter_sum_positive(&nr_files) >= files_stat.max_files)
+		if (percpu_counter_sum_positive(&nr_files) >= files_stat.max_files) {
+			error = -ENFILE;
 			goto over;
+		}
 	}
 
 	f = kmem_cache_zalloc(filp_cachep, GFP_KERNEL);
-	if (f == NULL)
+	if (f == NULL) {
+		error = -ENOMEM;
 		goto fail;
+	}
 
 	percpu_counter_inc(&nr_files);
 	f->f_cred = get_cred(cred);
-	if (security_file_alloc(f))
+	error = security_file_alloc(f);
+	if (error)
 		goto fail_sec;
 
 	INIT_LIST_HEAD(&f->f_u.fu_list);
@@ -149,7 +155,7 @@ over:
 fail_sec:
 	file_free(f);
 fail:
-	return NULL;
+	return ERR_PTR(error);
 }
 
 /**
@@ -173,8 +179,8 @@ struct file *alloc_file(struct path *path, fmode_t mode,
 	struct file *file;
 
 	file = get_empty_filp();
-	if (!file)
-		return NULL;
+	if (IS_ERR(file))
+		return file;
 
 	file->f_path = *path;
 	file->f_mapping = path->dentry->d_inode->i_mapping;
diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
index 8349a89..5ec849b 100644
--- a/fs/hugetlbfs/inode.c
+++ b/fs/hugetlbfs/inode.c
@@ -984,11 +984,12 @@ struct file *hugetlb_file_setup(const char *name, unsigned long addr,
 	inode->i_size = size;
 	clear_nlink(inode);
 
-	error = -ENFILE;
 	file = alloc_file(&path, FMODE_WRITE | FMODE_READ,
 			&hugetlbfs_file_operations);
-	if (!file)
+	if (IS_ERR(file)) {
+		error = PTR_ERR(file);
 		goto out_dentry; /* inode is already attached */
+	}
 
 	return file;
 
diff --git a/fs/namei.c b/fs/namei.c
index dd1ed1b..0160da0 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2885,8 +2885,8 @@ static struct file *path_openat(int dfd, const char *pathname,
 	int error;
 
 	file = get_empty_filp();
-	if (!file)
-		return ERR_PTR(-ENFILE);
+	if (IS_ERR(file))
+		return file;
 
 	file->f_flags = op->open_flag;
 
diff --git a/fs/open.c b/fs/open.c
index e1f2cdb..cb9a385 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -781,10 +781,9 @@ struct file *dentry_open(const struct path *path, int flags,
 	/* We must always pass in a valid mount pointer. */
 	BUG_ON(!path->mnt);
 
-	error = -ENFILE;
 	f = get_empty_filp();
-	if (f == NULL)
-		return ERR_PTR(error);
+	if (IS_ERR(f))
+		return f;
 
 	f->f_flags = flags;
 	f->f_path = *path;
diff --git a/fs/pipe.c b/fs/pipe.c
index 8d85d70..e420908 100644
--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -1035,16 +1035,19 @@ int create_pipe_files(struct file **res, int flags)
 
 	d_instantiate(path.dentry, inode);
 
-	err = -ENFILE;
 	f = alloc_file(&path, FMODE_WRITE, &write_pipefifo_fops);
-	if (!f)
+	if (IS_ERR(f)) {
+		err = PTR_ERR(f);
 		goto err_dentry;
+	}
 
 	f->f_flags = O_WRONLY | (flags & (O_NONBLOCK | O_DIRECT));
 
 	res[0] = alloc_file(&path, FMODE_READ, &read_pipefifo_fops);
-	if (!res[0])
+	if (IS_ERR(res[0])) {
+		err = PTR_ERR(res[0]);
 		goto err_file;
+	}
 
 	path_get(&path);
 	res[0]->f_flags = O_RDONLY | (flags & O_NONBLOCK);
diff --git a/ipc/shm.c b/ipc/shm.c
index 00faa05..c8eee21 100644
--- a/ipc/shm.c
+++ b/ipc/shm.c
@@ -1039,8 +1039,10 @@ long do_shmat(int shmid, char __user *shmaddr, int shmflg, ulong *raddr,
 			  is_file_hugepages(shp->shm_file) ?
 				&shm_file_operations_huge :
 				&shm_file_operations);
-	if (!file)
+	if (IS_ERR(file)) {
+		err = PTR_ERR(file);
 		goto out_free;
+	}
 
 	file->private_data = sfd;
 	file->f_mapping = shp->shm_file->f_mapping;
diff --git a/mm/shmem.c b/mm/shmem.c
index d4e184e..cd814ee 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -2948,11 +2948,12 @@ struct file *shmem_file_setup(const char *name, loff_t size, unsigned long flags
 		goto put_dentry;
 #endif
 
-	error = -ENFILE;
 	file = alloc_file(&path, FMODE_WRITE | FMODE_READ,
 		  &shmem_file_operations);
-	if (!file)
+	if (IS_ERR(file)) {
+		error = PTR_ERR(file);
 		goto put_dentry;
+	}
 
 	return file;
 
diff --git a/net/socket.c b/net/socket.c
index edc3c4a..dc1d071 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -369,12 +369,12 @@ static int sock_alloc_file(struct socket *sock, struct file **f, int flags)
 
 	file = alloc_file(&path, FMODE_READ | FMODE_WRITE,
 		  &socket_file_ops);
-	if (unlikely(!file)) {
+	if (unlikely(IS_ERR(file))) {
 		/* drop dentry, keep inode */
 		ihold(path.dentry->d_inode);
 		path_put(&path);
 		put_unused_fd(fd);
-		return -ENFILE;
+		return PTR_ERR(file);
 	}
 
 	sock->file = file;
-- 
1.7.12

--
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