fs/pipe.c: preserve alloc_file() error code
PATCH 1/2 fs/pipe.c: preserve alloc_file() error code
If sys_pipe() was unable to allocate a 'struct file', it always failed with ENFILE, which means "The number of simultaneously open files in the system would exceed a system-imposed limit." However, alloc_file() actually returns an ERR_PTR value and might fail with other error codes. Currently, in addition to ENFILE, it can fail with ENOMEM, potentially when there are few open files in the system. Update sys_pipe() to preserve this error code.
In a prior submission of a similar patch (1) some concern was raised about introducing a new error code for sys_pipe(). However, for most system calls, programs cannot assume that new error codes will never be introduced. In addition, ENOMEM was, in fact, already a possible error code for sys_pipe(), in the case where the file descriptor table could not be expanded due to insufficient memory.
(1) http://comments.gmane.org/gmane.linux.kernel/1357942
--- fs/pipe.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
fs/pipe.c +6 -3
@@ -693,17 +693,20 @@ int create_pipe_files(struct file **res, int flags)d_instantiate (path.dentry, inode); - err = -ENFILE ; f =alloc_file (&path,FMODE_WRITE , &pipefifo_fops); -if (IS_ERR (f)) +if (IS_ERR (f)) { + err =PTR_ERR (f); goto err_dentry; + } f->f_flags =O_WRONLY | (flags & (O_NONBLOCK |O_DIRECT )); f->private_data = inode->i_pipe; res[0 ] =alloc_file (&path,FMODE_READ , &pipefifo_fops); -if (IS_ERR (res[0 ])) +if (IS_ERR (res[0 ])) { + err =PTR_ERR (res[0 ]); goto err_file; + }path_get (&path); res[0 ]->private_data = inode->i_pipe;
Signature · 1 line
2.6.1
PATCH 2/2 fs/pipe.c: return error code rather than 0 in pipe_write()
pipe_write() would return 0 if it failed to merge the beginning of the data to write with the last, partially filled pipe buffer. It should return an error code instead. Userspace programs could be confused by write() returning 0 when called with a nonzero 'count'.
The EFAULT error case was a regression from f0d1bec9d5 ("new helper:copy_page_from_iter()"), while the ops->confirm() error case was a much older bug.
Test program:
#include <assert.h>
#include <errno.h>
#include <unistd.h>
int main(void)
{
int fd[2];
char data[1] = {0};
assert(0 == pipe(fd));
assert(1 == write(fd[1], data, 1));
/* prior to this patch, write() returned 0 here */
assert(-1 == write(fd[1], NULL, 1));
assert(errno == EFAULT);
}--- fs/pipe.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)
fs/pipe.c +4 -5
@@ -366,18 +366,17 @@ pipe_write(struct kiocb *iocb, struct iov_iter *from)int offset = buf->offset + buf->len;if (ops->can_merge && offset + chars <=PAGE_SIZE ) { -int error = ops->confirm (pipe, buf); -if (error) + ret = ops->confirm (pipe, buf); +if (ret) goto out; ret =copy_page_from_iter (buf->page, offset, chars, from);if (unlikely (ret < chars)) { - error = -EFAULT ; + ret = -EFAULT ; goto out; } do_wakeup =1 ; - buf->len += chars; - ret = chars; + buf->len += ret;if (!iov_iter_count (from)) goto out; }
Signature · 1 line
2.6.1