[bkpatch] add sys_sendfile64

[bkpatch] add sys_sendfile64

[Benjamin LaHaise]

The below bitkeeper patch can be pulled from
	bk://bcrlbits.bkbits.net/linux-2.5
and adds a sys_sendfile64 call.  Arch maintainers should 
probably add the entry to their syscall tables if it is 
appropriate.

		-ben
-- 
"A man with a bass just walked in,
 and he's putting it down
 on the floor."

# This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher.
# This patch includes the following deltas:
#	           ChangeSet	1.466   -> 1.467  
#	arch/i386/kernel/entry.S	1.19    -> 1.20   
#	        mm/filemap.c	1.63    -> 1.64   
#	include/asm-i386/unistd.h	1.6     -> 1.7    
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 02/03/03	bcrl@toomuch.toronto.redhat.com	1.467
# Add sendfile64 syscall to generic code and i386.
# --------------------------------------------
#
diff -Nru a/arch/i386/kernel/entry.S b/arch/i386/kernel/entry.S
--- a/arch/i386/kernel/entry.S	Sun Mar  3 16:15:55 2002
+++ b/arch/i386/kernel/entry.S	Sun Mar  3 16:15:55 2002
@@ -716,6 +716,7 @@
 	.long SYMBOL_NAME(sys_lremovexattr)
 	.long SYMBOL_NAME(sys_fremovexattr)
 	.long SYMBOL_NAME(sys_tkill)
+	.long SYMBOL_NAME(sys_sendfile64)
 
 	.rept NR_syscalls-(.-sys_call_table)/4
 		.long SYMBOL_NAME(sys_ni_syscall)
diff -Nru a/include/asm-i386/unistd.h b/include/asm-i386/unistd.h
--- a/include/asm-i386/unistd.h	Sun Mar  3 16:15:55 2002
+++ b/include/asm-i386/unistd.h	Sun Mar  3 16:15:55 2002
@@ -243,6 +243,7 @@
 #define __NR_lremovexattr	236
 #define __NR_fremovexattr	237
 #define __NR_tkill		238
+#define __NR_sendfile64		239
 
 /* user-visible error numbers are in the range -1 - -124: see <asm-i386/errno.h> */
 
diff -Nru a/mm/filemap.c b/mm/filemap.c
--- a/mm/filemap.c	Sun Mar  3 16:15:55 2002
+++ b/mm/filemap.c	Sun Mar  3 16:15:55 2002
@@ -1715,7 +1715,7 @@
 	return written;
 }
 
-asmlinkage ssize_t sys_sendfile(int out_fd, int in_fd, off_t *offset, size_t count)
+static ssize_t common_sendfile(int out_fd, int in_fd, loff_t *offset, size_t count)
 {
 	ssize_t retval;
 	struct file * in_file, * out_file;
@@ -1760,27 +1760,19 @@
 	retval = 0;
 	if (count) {
 		read_descriptor_t desc;
-		loff_t pos = 0, *ppos;
 
-		retval = -EFAULT;
-		ppos = &in_file->f_pos;
-		if (offset) {
-			if (get_user(pos, offset))
-				goto fput_out;
-			ppos = &pos;
-		}
+		if (!offset)
+			offset = &in_file->f_pos;
 
 		desc.written = 0;
 		desc.count = count;
 		desc.buf = (char *) out_file;
 		desc.error = 0;
-		do_generic_file_read(in_file, ppos, &desc, file_send_actor);
+		do_generic_file_read(in_file, offset, &desc, file_send_actor);
 
 		retval = desc.written;
 		if (!retval)
 			retval = desc.error;
-		if (offset)
-			put_user(pos, offset);
 	}
 
 fput_out:
@@ -1789,6 +1781,38 @@
 	fput(in_file);
 out:
 	return retval;
+}
+
+asmlinkage ssize_t sys_sendfile(int out_fd, int in_fd, off_t *offset, size_t count)
+{
+	loff_t pos, *ppos = NULL;
+	ssize_t ret;
+	if (offset) {
+		off_t off;
+		if (unlikely(get_user(off, offset)))
+			return -EFAULT;
+		pos = off;
+		ppos = &pos;
+	}
+	ret = common_sendfile(out_fd, in_fd, ppos, count);
+	if (offset)
+		put_user((off_t)pos, offset);
+	return ret;
+}
+
+asmlinkage ssize_t sys_sendfile64(int out_fd, int in_fd, loff_t *offset, size_t count)
+{
+	loff_t pos, *ppos = NULL;
+	ssize_t ret;
+	if (offset) {
+		if (unlikely(copy_from_user(&pos, offset, sizeof(loff_t))))
+			return -EFAULT;
+		ppos = &pos;
+	}
+	ret = common_sendfile(out_fd, in_fd, ppos, count);
+	if (offset)
+		put_user(pos, offset);
+	return ret;
 }
 
 static ssize_t do_readahead(struct file *file, unsigned long index, unsigned long nr)

Re: [bkpatch] add sys_sendfile64

[Chris Wedgwood]

On Sun, Mar 03, 2002 at 04:18:18PM -0500, Benjamin LaHaise wrote:

    The below bitkeeper patch can be pulled from
    	bk://bcrlbits.bkbits.net/linux-2.5

    and adds a sys_sendfile64 call.  Arch maintainers should 
    probably add the entry to their syscall tables if it is 
    appropriate.

Neat.  I made something similar last night but messed it up originally
so never sent it.

    +
    +asmlinkage ssize_t sys_sendfile(int out_fd, int in_fd, off_t *offset, size_t count)
    +{
    +	loff_t pos, *ppos = NULL;
    +	ssize_t ret;
    +	if (offset) {
    +		off_t off;
    +		if (unlikely(get_user(off, offset)))
    +			return -EFAULT;
    +		pos = off;
    +		ppos = &pos;

We have a problem if off + count >= 2^32 here.

Ideally i think we need to check for 32-bit (31-bit?) overflow here
and return -EOVERFLOW.  I made a similar patch last night for
sendfile64 which included this check (although I was tired and the
patch was slightly wrong).  Actually, I think wew are missing
EOVERFLOW checks in a number of paths, ideally I'd like to make one
function to check and have all other functions reference that if
people agree that makes sense.

    +	}
    +	ret = common_sendfile(out_fd, in_fd, ppos, count);
    +	if (offset)
    +		put_user((off_t)pos, offset);
    +	return ret;

What is another thread unmapped 'offset' during the system call?  Do
we want to check the result of put_user here and return -EFAULT?
(If so, there are other system calls to consider such as select).




  --cw

Re: [bkpatch] add sys_sendfile64

[Benjamin LaHaise]

On Sun, Mar 03, 2002 at 07:10:23PM -0800, Chris Wedgwood wrote:
> We have a problem if off + count >= 2^32 here.
> 
> Ideally i think we need to check for 32-bit (31-bit?) overflow here
> and return -EOVERFLOW.  I made a similar patch last night for
> sendfile64 which included this check (although I was tired and the
> patch was slightly wrong).  Actually, I think wew are missing
> EOVERFLOW checks in a number of paths, ideally I'd like to make one
> function to check and have all other functions reference that if
> people agree that makes sense.

I was just following the semantics of the original code.  -EOVERFLOW 
checks are certainly doable; I'll post an update in a bit.

>     +	}
>     +	ret = common_sendfile(out_fd, in_fd, ppos, count);
>     +	if (offset)
>     +		put_user((off_t)pos, offset);
>     +	return ret;
> 
> What is another thread unmapped 'offset' during the system call?  Do
> we want to check the result of put_user here and return -EFAULT?
> (If so, there are other system calls to consider such as select).

Again, the original code didn't bother checking.  As far as how it 
should work, I'd rather send a segv to the app as otherwise it is 
impossible to determine how much data was actually transferred.

		-ben

Re: [bkpatch] add sys_sendfile64

[Benjamin LaHaise]

Okay, here are the checks for overflow and page faults.  Again, pull from 

	bk://bcrlbits.bkbits.net/linux-2.5

To grab the updates, or apply this on top of the previous patch.

		-ben
--
"A man with a bass just walked in,
 and he's putting it down
 on the floor."

# This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher.
# This patch includes the following deltas:
#	           ChangeSet	1.467   -> 1.468  
#	        mm/filemap.c	1.64    -> 1.65   
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 02/03/04	bcrl@toomuch.toronto.redhat.com	1.468
# Add LFS style EOVERFLOW checks to sendfile*
# --------------------------------------------
#
diff -Nru a/mm/filemap.c b/mm/filemap.c
--- a/mm/filemap.c	Mon Mar  4 11:25:28 2002
+++ b/mm/filemap.c	Mon Mar  4 11:25:28 2002
@@ -1715,7 +1715,7 @@
 	return written;
 }
 
-static ssize_t common_sendfile(int out_fd, int in_fd, loff_t *offset, size_t count)
+static ssize_t common_sendfile(int out_fd, int in_fd, loff_t *offset, size_t count, loff_t max)
 {
 	ssize_t retval;
 	struct file * in_file, * out_file;
@@ -1760,10 +1760,22 @@
 	retval = 0;
 	if (count) {
 		read_descriptor_t desc;
+		loff_t pos;
 
 		if (!offset)
 			offset = &in_file->f_pos;
 
+		pos = *offset;
+		retval = -EINVAL;
+		if (unlikely(pos < 0))
+			goto fput_out;
+		if (unlikely(pos + count > max)) {
+			retval = -EOVERFLOW;
+			if (pos >= max)
+				goto fput_out;
+			count = max - pos;
+		}
+
 		desc.written = 0;
 		desc.count = count;
 		desc.buf = (char *) out_file;
@@ -1773,6 +1785,9 @@
 		retval = desc.written;
 		if (!retval)
 			retval = desc.error;
+		pos = *offset;
+		if (pos > max)
+			retval = -EOVERFLOW;
 	}
 
 fput_out:
@@ -1794,9 +1809,9 @@
 		pos = off;
 		ppos = &pos;
 	}
-	ret = common_sendfile(out_fd, in_fd, ppos, count);
-	if (offset)
-		put_user((off_t)pos, offset);
+	ret = common_sendfile(out_fd, in_fd, ppos, count, MAX_NON_LFS);
+	if (offset && put_user(pos, offset))
+		ret = -EFAULT;
 	return ret;
 }
 
@@ -1809,9 +1824,9 @@
 			return -EFAULT;
 		ppos = &pos;
 	}
-	ret = common_sendfile(out_fd, in_fd, ppos, count);
-	if (offset)
-		put_user(pos, offset);
+	ret = common_sendfile(out_fd, in_fd, ppos, count, MAX_LFS_FILESIZE);
+	if (offset && put_user(pos, offset))
+		ret = -EFAULT;
 	return ret;
 }