| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <m1y6s64glm.fsf@fess.ebiederm.org>
Date: Fri, 05 Jun 2009 12:37:25 -0700
From: ebiederm@...ssion.com (Eric W. Biederman)
To: Badari Pulavarty <pbadari@...il.com>
Cc: Al Viro <viro@...IV.linux.org.uk>, linux-kernel@...r.kernel.org,
linux-pci@...r.kernel.org, linux-mm@...ck.org,
linux-fsdevel@...r.kernel.org, Hugh Dickins <hugh@...itas.com>,
Tejun Heo <tj@...nel.org>,
Alexey Dobriyan <adobriyan@...il.com>,
Linus Torvalds <torvalds@...ux-foundation.org>,
Alan Cox <alan@...rguk.ukuu.org.uk>,
Greg Kroah-Hartman <gregkh@...e.de>,
Nick Piggin <npiggin@...e.de>,
Andrew Morton <akpm@...ux-foundation.org>,
Christoph Hellwig <hch@...radead.org>,
"Eric W. Biederman" <ebiederm@...well.arastra.com>,
"Eric W. Biederman" <ebiederm@...stanetworks.com>
Subject: Re: [PATCH 07/23] vfs: Teach sendfile,splice,tee,and vmsplice to use file_hotplug_lock
Badari Pulavarty <pbadari@...il.com> writes:
> On Mon, 2009-06-01 at 14:50 -0700, Eric W. Biederman wrote:
>> From: Eric W. Biederman <ebiederm@...well.arastra.com>
>>
>> Signed-off-by: Eric W. Biederman <ebiederm@...stanetworks.com>
>> ---
>> 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 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