From mboxrd@z Thu Jan 1 00:00:00 1970 From: ebiederm@xmission.com (Eric W. Biederman) Subject: Re: [PATCH 07/23] vfs: Teach sendfile,splice,tee,and vmsplice to use file_hotplug_lock Date: Fri, 05 Jun 2009 12:37:25 -0700 Message-ID: References: <1243893048-17031-7-git-send-email-ebiederm@xmission.com> <1244072363.6383.15.camel@badari-desktop> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Al Viro , linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org, linux-mm@kvack.org, linux-fsdevel@vger.kernel.org, Hugh Dickins , Tejun Heo , Alexey Dobriyan , Linus Torvalds , Alan Cox , Greg Kroah-Hartman , Nick Piggin , Andrew Morton , Christoph Hellwig , "Eric W. Biederman" , "Eric W. Biederman" To: Badari Pulavarty Return-path: In-Reply-To: <1244072363.6383.15.camel@badari-desktop> (Badari Pulavarty's message of "Wed\, 03 Jun 2009 16\:39\:23 -0700") Sender: owner-linux-mm@kvack.org List-Id: linux-fsdevel.vger.kernel.org Badari Pulavarty writes: > On Mon, 2009-06-01 at 14:50 -0700, Eric W. Biederman wrote: >> From: Eric W. Biederman >> >> Signed-off-by: Eric W. Biederman >> --- >> fs/read_write.c | 28 +++++++++---- >> fs/splice.c | 111 +++++++++++++++++++++++++++++++++++++----------------- >> 2 files changed, 95 insertions(+), 44 deletions(-) >> >> diff --git a/fs/read_write.c b/fs/read_write.c >> index 718baea..c473d74 100644 >> --- a/fs/read_write.c >> +++ b/fs/read_write.c >> @@ -861,21 +861,24 @@ static ssize_t do_sendfile(int out_fd, int in_fd, loff_t *ppos, >> goto out; >> if (!(in_file->f_mode & FMODE_READ)) >> goto fput_in; >> + retval = -EIO; >> + if (!file_hotplug_read_trylock(in_file)) >> + goto fput_in; >> retval = -EINVAL; >> in_inode = in_file->f_path.dentry->d_inode; >> if (!in_inode) >> - goto fput_in; >> + goto unlock_in; >> if (!in_file->f_op || !in_file->f_op->splice_read) >> - goto fput_in; >> + goto unlock_in; >> retval = -ESPIPE; >> if (!ppos) >> ppos = &in_file->f_pos; >> else >> if (!(in_file->f_mode & FMODE_PREAD)) >> - goto fput_in; >> + goto unlock_in; >> retval = rw_verify_area(READ, in_file, ppos, count); >> if (retval < 0) >> - goto fput_in; >> + goto unlock_in; >> count = retval; >> >> /* >> @@ -884,16 +887,19 @@ static ssize_t do_sendfile(int out_fd, int in_fd, loff_t *ppos, >> retval = -EBADF; >> out_file = fget_light(out_fd, &fput_needed_out); >> if (!out_file) >> - goto fput_in; >> + goto unlock_in; >> if (!(out_file->f_mode & FMODE_WRITE)) >> goto fput_out; >> + retval = -EIO; >> + if (!file_hotplug_read_trylock(out_file)) >> + goto fput_out; >> retval = -EINVAL; >> if (!out_file->f_op || !out_file->f_op->sendpage) >> - goto fput_out; >> + goto unlock_out; >> out_inode = out_file->f_path.dentry->d_inode; >> retval = rw_verify_area(WRITE, out_file, &out_file->f_pos, count); >> if (retval < 0) >> - goto fput_out; >> + goto unlock_out; >> count = retval; >> >> if (!max) >> @@ -902,11 +908,11 @@ static ssize_t do_sendfile(int out_fd, int in_fd, loff_t *ppos, >> pos = *ppos; >> retval = -EINVAL; >> if (unlikely(pos < 0)) >> - goto fput_out; >> + goto unlock_out; >> if (unlikely(pos + count > max)) { >> retval = -EOVERFLOW; >> if (pos >= max) >> - goto fput_out; >> + goto unlock_out; >> count = max - pos; >> } >> >> @@ -933,8 +939,12 @@ static ssize_t do_sendfile(int out_fd, int in_fd, loff_t *ppos, >> if (*ppos > max) >> retval = -EOVERFLOW; >> >> +unlock_out: >> + file_hotplug_read_unlock(out_file); >> fput_out: >> fput_light(out_file, fput_needed_out); >> +unlock_in: >> + file_hotplug_read_unlock(in_file); >> fput_in: >> fput_light(in_file, fput_needed_in); >> out: >> diff --git a/fs/splice.c b/fs/splice.c >> index 666953d..fc6b3a5 100644 >> --- a/fs/splice.c >> +++ b/fs/splice.c >> @@ -1464,15 +1464,21 @@ SYSCALL_DEFINE4(vmsplice, int, fd, const struct iovec __user *, iov, >> >> error = -EBADF; >> file = fget_light(fd, &fput); >> - if (file) { >> - if (file->f_mode & FMODE_WRITE) >> - error = vmsplice_to_pipe(file, iov, nr_segs, flags); >> - else if (file->f_mode & FMODE_READ) >> - error = vmsplice_to_user(file, iov, nr_segs, flags); >> + if (!file) >> + goto out; >> >> - fput_light(file, fput); >> - } >> + if (!file_hotplug_read_trylock(file)) >> + goto fput_file; >> >> + if (file->f_mode & FMODE_WRITE) >> + error = vmsplice_to_pipe(file, iov, nr_segs, flags); >> + else if (file->f_mode & FMODE_READ) >> + error = vmsplice_to_user(file, iov, nr_segs, flags); >> + >> + file_hotplug_read_unlock(file); >> +fput_file: >> + fput_light(file, fput); >> +out: >> return error; >> } >> >> @@ -1489,21 +1495,39 @@ SYSCALL_DEFINE6(splice, int, fd_in, loff_t __user *, off_in, >> >> error = -EBADF; >> in = fget_light(fd_in, &fput_in); >> - if (in) { >> - if (in->f_mode & FMODE_READ) { >> - out = fget_light(fd_out, &fput_out); >> - if (out) { >> - if (out->f_mode & FMODE_WRITE) >> - error = do_splice(in, off_in, >> - out, off_out, >> - len, flags); >> - fput_light(out, fput_out); >> - } >> - } >> + if (!in) >> + goto out; >> >> - fput_light(in, fput_in); >> - } >> + if (!(in->f_mode & FMODE_READ)) >> + goto fput_in; >> + >> + error = -EIO; >> + if (!file_hotplug_read_trylock(in)) >> + goto fput_in; >> + >> + error = -EBADF; >> + out = fget_light(fd_out, &fput_out); >> + if (!out) >> + goto unlock_in; >> + >> + if (!(out->f_mode & FMODE_WRITE)) >> + goto fput_out; >> + >> + error = -EIO; >> + if (!file_hotplug_read_trylock(out)) >> + goto fput_out; >> + >> + error = do_splice(in, off_in, out, off_out, len, flags); >> >> + file_hotplug_read_unlock(out); >> +fput_out: >> + fput_light(out, fput_out); >> +unlock_in: >> + file_hotplug_read_unlock(in); >> +fput_in: >> + fput_light(in, fput_in); >> + >> +out: >> return error; >> } >> >> @@ -1703,27 +1727,44 @@ static long do_tee(struct file *in, struct file *out, size_t len, >> >> SYSCALL_DEFINE4(tee, int, fdin, int, fdout, size_t, len, unsigned int, flags) >> { >> - struct file *in; >> - int error, fput_in; >> + struct file *in, *out; >> + int error, fput_in, fput_out; >> >> if (unlikely(!len)) >> return 0; >> >> error = -EBADF; >> in = fget_light(fdin, &fput_in); >> - if (in) { >> - if (in->f_mode & FMODE_READ) { >> - int fput_out; >> - struct file *out = fget_light(fdout, &fput_out); >> - >> - if (out) { >> - if (out->f_mode & FMODE_WRITE) >> - error = do_tee(in, out, len, flags); >> - fput_light(out, fput_out); >> - } >> - } >> - fput_light(in, fput_in); >> - } >> + if (!in) >> + goto out; >> + >> + if (!(in->f_mode & FMODE_READ)) >> + goto unlock_in; <<<<<<< > > Shouldn't this be > goto fput_in; Good point. That is a bug. > ? btw, its confusing to have labels and variables with same name: > fput_in and fput_out. You may want to rename labels ? Mayhap. I didn't start that one, although I am clearly spreading it around here. Do you have a better naming suggestion? >> + error = -EIO; >> + if (!file_hotplug_read_trylock(in)) >> + goto fput_in; >> + >> + error = -EBADF; >> + out = fget_light(fdout, &fput_out); >> + if (!out) >> + goto unlock_in; >> + >> + if (!(out->f_mode & FMODE_WRITE)) >> + goto fput_out; >> + >> + if (!file_hotplug_read_trylock(out)) >> + goto fput_out; >> + >> + error = do_tee(in, out, len, flags); >> + >> + file_hotplug_read_unlock(out); >> +fput_out: >> + fput_light(out, fput_out); >> +unlock_in: >> + file_hotplug_read_unlock(in); >> +fput_in: >> + fput_light(in, fput_in); >> +out: >> return error; >> } Eric -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org