diff mbox

[v2] lseek: Correct order of offset arguments

Message ID 1406917127-8088-1-git-send-email-Anton.Kolesov@synopsys.com
State Accepted
Headers show

Commit Message

Anton Kolesov Aug. 1, 2014, 6:18 p.m. UTC
There was a runtime error in systems without large file support. Call
fseek(fd, 4096, SEEK_SET) has been failing with EINVAL, though it was
succeeding for offset = 4092. This has been happening because llseek system
call accepts 64-bit value as an offset argument and lseek function has been
ordering 32-bits words that form this offset value, according to the
endianness. However this ordering to match endianness is not required,
because llseek doesn't accept one 64-bit offset argument, it accepts two
32-bit offset argument, then stitches them into one following its
endianness. As a result on little endian system, order of words has been
swapped two time: in libc and in kernel. Thus call to fseek with offset 4096
(0x1000) was doing a system call to llseek with offset 0x1000_0000_0000. I'm
not entirely sure why then offset = 4092 hasn't been failing then.

This patch removes malicious swap of words when calling llseek.

Signed-off-by: Anton Kolesov <Anton.Kolesov@synopsys.com>
---
 libc/sysdeps/linux/common/lseek.c |  4 +---
 test/stdio/lseek_no_lfs.c         | 22 ++++++++++++++++++++++
 2 files changed, 23 insertions(+), 3 deletions(-)
 create mode 100644 test/stdio/lseek_no_lfs.c

Comments

Bernhard Reutner-Fischer Aug. 18, 2014, 10:03 a.m. UTC | #1
On Fri, Aug 01, 2014 at 10:18:47PM +0400, Anton Kolesov wrote:
> There was a runtime error in systems without large file support. Call
> fseek(fd, 4096, SEEK_SET) has been failing with EINVAL, though it was
> succeeding for offset = 4092. This has been happening because llseek system
> call accepts 64-bit value as an offset argument and lseek function has been
> ordering 32-bits words that form this offset value, according to the
> endianness. However this ordering to match endianness is not required,
> because llseek doesn't accept one 64-bit offset argument, it accepts two
> 32-bit offset argument, then stitches them into one following its
> endianness. As a result on little endian system, order of words has been
> swapped two time: in libc and in kernel. Thus call to fseek with offset 4096
> (0x1000) was doing a system call to llseek with offset 0x1000_0000_0000. I'm
> not entirely sure why then offset = 4092 hasn't been failing then.
> 
> This patch removes malicious swap of words when calling llseek.

Applied, thanks!
diff mbox

Patch

diff --git a/libc/sysdeps/linux/common/lseek.c b/libc/sysdeps/linux/common/lseek.c
index 500c6bf..11a1fbb 100644
--- a/libc/sysdeps/linux/common/lseek.c
+++ b/libc/sysdeps/linux/common/lseek.c
@@ -24,9 +24,7 @@  off_t __NC(lseek)(int fd, off_t offset, int whence)
 #elif __WORDSIZE == 32
 	__off64_t result;
 	__off_t high = 0;
-	return INLINE_SYSCALL(llseek, 5, fd,
-				__LONG_LONG_PAIR(high, offset),
-				&result, whence) ?: result;
+	return INLINE_SYSCALL(llseek, 5, fd, high, offset, &result, whence) ?: result;
 #endif
 /* No need to handle __WORDSIZE == 64 as such a kernel won't define __NR_llseek */
 }
diff --git a/test/stdio/lseek_no_lfs.c b/test/stdio/lseek_no_lfs.c
new file mode 100644
index 0000000..54daf6b
--- /dev/null
+++ b/test/stdio/lseek_no_lfs.c
@@ -0,0 +1,22 @@ 
+#include <stdio.h>
+#include <string.h>
+#include <errno.h>
+
+int main(int argc, char *argv[])
+{
+    FILE * f = fopen(argv[0], "rb");
+    if (!f)
+      {
+	printf("Error: Can't open %s, reason: %s\n", argv[0], strerror(errno));
+	return 1;
+      }
+
+    if (fseek(f, (unsigned)4096, (int)SEEK_SET) == -1)
+      {
+	printf("Test failed, fseek return fail code. errno=%u (%s)\n", errno, strerror(errno));
+	return 1;
+      }
+
+    fclose(f);
+    return 0;
+}