linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] fs: Preserve error code in get_empty_filp()
@ 2012-09-13  3:11 Anatol Pomozov
  2012-10-05 18:16 ` Anatol Pomozov
  0 siblings, 1 reply; 12+ messages in thread
From: Anatol Pomozov @ 2012-09-13  3:11 UTC (permalink / raw)
  To: linux-fsdevel, viro, linux-kernel; +Cc: tytso, akpm, Anatol Pomozov

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@gmail.com>
Reviewed-by: "Theodore Ts'o" <tytso@mit.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


^ permalink raw reply related	[flat|nested] 12+ messages in thread
* [PATCH] fs: Preserve error code in get_empty_filp()
@ 2012-08-01 18:19 anatol.pomozov
  2012-08-01 18:34 ` anatol.pomozov
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: anatol.pomozov @ 2012-08-01 18:19 UTC (permalink / raw)
  To: linux-kernel; +Cc: viro, tytso, Anatol Pomazau, Anatol Pomozov

From: Anatol Pomazau <anatol@google.com>

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@gmail.com>
Reviewed-by: "Theodore Ts'o" <tytso@mit.edu>
---
 arch/ia64/kernel/perfmon.c |  4 ++--
 fs/anon_inodes.c           |  5 +++--
 fs/file_table.c            | 23 +++++++++++++++--------
 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(+), 28 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 b3fc4d6..5e65706 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 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,18 +118,24 @@ 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))
+	if (security_file_alloc(f)) {
+		error = -EPERM;
 		goto fail_sec;
+	}
 
 	INIT_LIST_HEAD(&f->f_u.fu_list);
 	atomic_long_set(&f->f_count, 1);
@@ -149,7 +156,7 @@ over:
 fail_sec:
 	file_free(f);
 fail:
-	return NULL;
+	return ERR_PTR(error);
 }
 
 /**
@@ -173,8 +180,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 2ccc35c..b77bdca 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2753,8 +2753,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 1e914b3..195bdab 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -777,10 +777,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 95cbd6b..429f2db 100644
--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -1037,10 +1037,11 @@ struct file *create_write_pipe(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_mapping = inode->i_mapping;
 
 	f->f_flags = O_WRONLY | (flags & (O_NONBLOCK | O_DIRECT));
@@ -1072,8 +1073,8 @@ struct file *create_read_pipe(struct file *wrf, int flags)
 	/* Grab pipe from the writer */
 	struct file *f = alloc_file(&wrf->f_path, FMODE_READ,
 				    &read_pipefifo_fops);
-	if (!f)
-		return ERR_PTR(-ENFILE);
+	if (IS_ERR(f))
+		return f;
 
 	path_get(&wrf->f_path);
 	f->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 dfe5b66..e80eb83 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.11.3


^ permalink raw reply related	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2013-02-21 16:36 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-09-13  3:11 [PATCH] fs: Preserve error code in get_empty_filp() Anatol Pomozov
2012-10-05 18:16 ` Anatol Pomozov
2012-10-05 18:39   ` Theodore Ts'o
2013-02-15  2:21   ` Al Viro
2013-02-21 16:36     ` Theodore Ts'o
  -- strict thread matches above, loose matches on Subject: below --
2012-08-01 18:19 anatol.pomozov
2012-08-01 18:34 ` anatol.pomozov
2012-08-03  0:30   ` Anatol Pomozov
2012-08-03  0:47 ` Anatol Pomozov
2012-09-04 21:15   ` Anatol Pomozov
2012-08-21 10:06 ` Jan Engelhardt
2012-08-21 13:51   ` Anatol Pomozov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).