Skip to contentSkip to conversation

add sys_sendfile64

bkpatch Benjamin LaHaise 2 participants 4 messages Mar 3, 2002 – Mar 4, 2002

Subsystems (2)
Memory management Page cache
lore
Share & tools
lore

Benjamin LaHaise

author
#0 lore raw

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

Signature · 123 lines
"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	[email protected]	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)
#1 lore raw

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

author
#2 lore raw
In reply to Benjamin LaHaise
Quoted 9 lines · Chris Wedgwood

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.

Quoted 9 lines
    +	}
    +	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

author
#3 lore raw
In reply to 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 [email protected] 1.468
# Add LFS style EOVERFLOW checks to sendfile*
# --------------------------------------------
#

mm/filemap.c +22 -7
diff -Nru a/mm/filemap.c b/mm/filemap.c
@@ -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;
 }