move O_LARGEFILE forcing to filp_open()

move O_LARGEFILE forcing to filp_open()

[William Lee Irwin III]

Internal kernel open() of files barfs in important contexts, for
instance, using strict non-overcommit with enough swap for large
commitments. This is carried out through the entrypoint filp_open(),
not sys_open(). sys_open() in turn calls filp_open(). So merely
moving the forcing of the flag on 64-bit resolves this situation there,
though not for 32-bit, whose solution is to appear in the sequel.


-- wli

Index: mm5-2.6.7/fs/open.c
===================================================================
--- mm5-2.6.7.orig/fs/open.c	2004-06-15 22:18:56.000000000 -0700
+++ mm5-2.6.7/fs/open.c	2004-07-03 22:58:51.081134896 -0700
@@ -755,6 +755,8 @@
 	int namei_flags, error;
 	struct nameidata nd;
 
+	if (BITS_PER_LONG > 32)
+		flags |= O_LARGEFILE;
 	namei_flags = flags;
 	if ((namei_flags+1) & O_ACCMODE)
 		namei_flags++;
@@ -943,9 +945,6 @@
 	char * tmp;
 	int fd, error;
 
-#if BITS_PER_LONG != 32
-	flags |= O_LARGEFILE;
-#endif
 	tmp = getname(filename);
 	fd = PTR_ERR(tmp);
 	if (!IS_ERR(tmp)) {

force O_LARGEFILE in sys_swapon() and sys_swapoff()

[William Lee Irwin III]

On Sat, Jul 03, 2004 at 11:41:22PM -0700, William Lee Irwin III wrote:
> Internal kernel open() of files barfs in important contexts, for
> instance, using strict non-overcommit with enough swap for large
> commitments. This is carried out through the entrypoint filp_open(),
> not sys_open(). sys_open() in turn calls filp_open(). So merely
> moving the forcing of the flag on 64-bit resolves this situation there,
> though not for 32-bit, whose solution is to appear in the sequel.

For 32-bit, one quickly discovers that swapon() is not given an fd
already opened with O_LARGEFILE to act upon and the forcing of
O_LARGEFILE for 64-bit is irrelevant, as the system call's argument is
a path. So this patch manually forces it for swapon() and swapoff().


-- wli

Index: mm5-2.6.7/mm/swapfile.c
===================================================================
--- mm5-2.6.7.orig/mm/swapfile.c	2004-07-02 20:43:30.000000000 -0700
+++ mm5-2.6.7/mm/swapfile.c	2004-07-03 23:12:35.000000000 -0700
@@ -1085,7 +1085,7 @@
 	if (IS_ERR(pathname))
 		goto out;
 
-	victim = filp_open(pathname, O_RDWR, 0);
+	victim = filp_open(pathname, O_RDWR|O_LARGEFILE, 0);
 	putname(pathname);
 	err = PTR_ERR(victim);
 	if (IS_ERR(victim))
@@ -1354,7 +1354,7 @@
 		name = NULL;
 		goto bad_swap_2;
 	}
-	swap_file = filp_open(name, O_RDWR, 0);
+	swap_file = filp_open(name, O_RDWR|O_LARGEFILE, 0);
 	error = PTR_ERR(swap_file);
 	if (IS_ERR(swap_file)) {
 		swap_file = NULL;

Re: force O_LARGEFILE in sys_swapon() and sys_swapoff()

[Hugh Dickins]

On Sat, 3 Jul 2004, William Lee Irwin III wrote:
> 
> For 32-bit, one quickly discovers that swapon() is not given an fd
> already opened with O_LARGEFILE to act upon and the forcing of
> O_LARGEFILE for 64-bit is irrelevant, as the system call's argument is
> a path. So this patch manually forces it for swapon() and swapoff().

This one looks good, thanks.  I'm not so sure of your more general
patch to open, others know better on that.

I doubt huge amounts of swap work at all well when used, but you're
really concerned with commit at present: of course it was silly not
to have used O_LARGEFILE here.

Hugh

Re: force O_LARGEFILE in sys_swapon() and sys_swapoff()

[William Lee Irwin III]

On Sat, 3 Jul 2004, William Lee Irwin III wrote:
>> For 32-bit, one quickly discovers that swapon() is not given an fd
>> already opened with O_LARGEFILE to act upon and the forcing of
>> O_LARGEFILE for 64-bit is irrelevant, as the system call's argument is
>> a path. So this patch manually forces it for swapon() and swapoff().

On Sun, Jul 04, 2004 at 08:21:25PM +0100, Hugh Dickins wrote:
> This one looks good, thanks.  I'm not so sure of your more general
> patch to open, others know better on that.
> I doubt huge amounts of swap work at all well when used, but you're
> really concerned with commit at present: of course it was silly not
> to have used O_LARGEFILE here.

The story on the more general one is that it silently broke -EFBIG
returns for 32-bit emulation. Arnd posted a patch to consolidate 32-bit
open() emulation, which I followed up with a hoist of the O_LARGEFILE
check to there above filp_open(). The forcing of O_LARGEFILE is still
needed for other, non-open()-related situations, and I may need to
sweep 32-bit emulation for other syscalls. Of course, it's a lose-lose
situation, as 64-bit now can't do internal kernel filp_open() on files
larger than 64-bit unsafe 32-bit apps can. In short, a large headache.

-- wli

Re: move O_LARGEFILE forcing to filp_open()

[Arnd Bergmann]

On Sonntag, 4. Juli 2004 08:41, William Lee Irwin III wrote:
>                        sys_open() in turn calls filp_open(). So merely
> moving the forcing of the flag on 64-bit resolves this situation there,
> though not for 32-bit, whose solution is to appear in the sequel.

Unfortunately, this will also cause problems for 32-bit emulation, where
sys32_open currently calls filp_open without forcing O_LARGEFILE for
32 bit applications.
I also noticed that this behavior currently is not implemented on all
architectures, which in turn would need the patch below.

Maybe you can find a way to fix both problems.

	Arnd <><

 arch/ia64/ia32/ia32_entry.S         |    2 +-
 arch/ia64/ia32/sys_ia32.c           |   31 -------------------------------
 arch/mips/kernel/scall64-o32.S      |    2 +-
 arch/parisc/kernel/syscall_table.S  |    2 +-
 arch/ppc64/kernel/misc.S            |    2 +-
 arch/ppc64/kernel/sys_ppc32.c       |   31 -------------------------------
 arch/s390/kernel/compat_wrapper.S   |    2 +-
 arch/sparc64/kernel/sparc64_ksyms.c |    2 --
 arch/sparc64/kernel/sys32.S         |    2 +-
 arch/sparc64/kernel/sys_sparc32.c   |   32 --------------------------------
 arch/sparc64/kernel/sys_sunos32.c   |    4 +---
 arch/sparc64/solaris/fs.c           |    4 +---
 arch/x86_64/ia32/ia32entry.S        |    2 +-
 arch/x86_64/ia32/sys_ia32.c         |   24 ------------------------
 fs/compat.c                         |   31 +++++++++++++++++++++++++++++++
 include/linux/compat.h              |    4 ++++
 16 files changed, 44 insertions(+), 133 deletions(-)

===== arch/ia64/ia32/ia32_entry.S 1.39 vs edited =====
--- 1.39/arch/ia64/ia32/ia32_entry.S	Sun Jun 27 08:06:52 2004
+++ edited/arch/ia64/ia32/ia32_entry.S	Sun Jul  4 14:02:04 2004
@@ -213,7 +213,7 @@
 	data8 sys32_fork
 	data8 sys_read
 	data8 sys_write
-	data8 sys32_open	  /* 5 */
+	data8 compat_sys_open	  /* 5 */
 	data8 sys_close
 	data8 sys32_waitpid
 	data8 sys_creat
===== arch/ia64/ia32/sys_ia32.c 1.100 vs edited =====
--- 1.100/arch/ia64/ia32/sys_ia32.c	Sun Jun 27 08:06:52 2004
+++ edited/arch/ia64/ia32/sys_ia32.c	Sun Jul  4 13:28:46 2004
@@ -2450,37 +2450,6 @@
 	return ret;
 }
 
-/*
- * Exactly like fs/open.c:sys_open(), except that it doesn't set the O_LARGEFILE flag.
- */
-asmlinkage long
-sys32_open (const char * filename, int flags, int mode)
-{
-	char * tmp;
-	int fd, error;
-
-	tmp = getname(filename);
-	fd = PTR_ERR(tmp);
-	if (!IS_ERR(tmp)) {
-		fd = get_unused_fd();
-		if (fd >= 0) {
-			struct file *f = filp_open(tmp, flags, mode);
-			error = PTR_ERR(f);
-			if (IS_ERR(f))
-				goto out_error;
-			fd_install(fd, f);
-		}
-out:
-		putname(tmp);
-	}
-	return fd;
-
-out_error:
-	put_unused_fd(fd);
-	fd = error;
-	goto out;
-}
-
 /* Structure for ia32 emulation on ia64 */
 struct epoll_event32
 {
===== arch/mips/kernel/scall64-o32.S 1.6 vs edited =====
--- 1.6/arch/mips/kernel/scall64-o32.S	Sun Jun 27 08:06:52 2004
+++ edited/arch/mips/kernel/scall64-o32.S	Sun Jul  4 14:05:34 2004
@@ -263,7 +263,7 @@
 	sys	sys_fork	0
 	sys	sys_read	3
 	sys	sys_write	3
-	sys	sys_open	3			/* 4005 */
+	sys	compat_sys_open	3			/* 4005 */
 	sys	sys_close	1
 	sys	sys_waitpid	3
 	sys	sys_creat	2
===== arch/parisc/kernel/syscall_table.S 1.8 vs edited =====
--- 1.8/arch/parisc/kernel/syscall_table.S	Mon May 10 11:18:39 2004
+++ edited/arch/parisc/kernel/syscall_table.S	Sun Jul  4 14:03:36 2004
@@ -65,7 +65,7 @@
 	ENTRY_SAME(fork_wrapper)
 	ENTRY_SAME(read)
 	ENTRY_SAME(write)
-	ENTRY_SAME(open)		/* 5 */
+	ENTRY_COMP(open)		/* 5 */
 	ENTRY_SAME(close)
 	ENTRY_SAME(waitpid)
 	ENTRY_SAME(creat)
===== arch/ppc64/kernel/misc.S 1.83 vs edited =====
--- 1.83/arch/ppc64/kernel/misc.S	Sun Jun 27 08:06:53 2004
+++ edited/arch/ppc64/kernel/misc.S	Sun Jul  4 14:01:27 2004
@@ -612,7 +612,7 @@
 	.llong .ppc_fork
 	.llong .sys_read
 	.llong .sys_write
-	.llong .sys32_open		/* 5 */
+	.llong .compat_sys_open		/* 5 */
 	.llong .sys_close
 	.llong .sys32_waitpid
 	.llong .sys32_creat
===== arch/ppc64/kernel/sys_ppc32.c 1.95 vs edited =====
--- 1.95/arch/ppc64/kernel/sys_ppc32.c	Sun Jun 27 08:06:53 2004
+++ edited/arch/ppc64/kernel/sys_ppc32.c	Sun Jul  4 13:29:18 2004
@@ -905,37 +905,6 @@
 	return sys_lseek(fd, (int)offset, origin);
 }
 
-/*
- * This is just a version for 32-bit applications which does
- * not force O_LARGEFILE on.
- */
-asmlinkage long sys32_open(const char __user * filename, int flags, int mode)
-{
-	char * tmp;
-	int fd, error;
-
-	tmp = getname(filename);
-	fd = PTR_ERR(tmp);
-	if (!IS_ERR(tmp)) {
-		fd = get_unused_fd();
-		if (fd >= 0) {
-			struct file * f = filp_open(tmp, flags, mode);
-			error = PTR_ERR(f);
-			if (IS_ERR(f))
-				goto out_error;
-			fd_install(fd, f);
-		}
-out:
-		putname(tmp);
-	}
-	return fd;
-
-out_error:
-	put_unused_fd(fd);
-	fd = error;
-	goto out;
-}
-
 /* Note: it is necessary to treat bufsiz as an unsigned int,
  * with the corresponding cast to a signed int to insure that the 
  * proper conversion (sign extension) between the register representation of a signed int (msr in 32-bit mode)
===== arch/s390/kernel/compat_wrapper.S 1.16 vs edited =====
--- 1.16/arch/s390/kernel/compat_wrapper.S	Wed Jun  9 10:47:16 2004
+++ edited/arch/s390/kernel/compat_wrapper.S	Sun Jul  4 13:25:22 2004
@@ -32,7 +32,7 @@
 	llgtr	%r2,%r2			# const char *
 	lgfr	%r3,%r3			# int
 	lgfr	%r4,%r4			# int
-	jg	sys_open		# branch to system call
+	jg	compat_sys_open		# branch to system call
 
 	.globl  sys32_close_wrapper 
 sys32_close_wrapper:
===== arch/sparc64/kernel/sparc64_ksyms.c 1.70 vs edited =====
--- 1.70/arch/sparc64/kernel/sparc64_ksyms.c	Sun Jun 27 08:06:53 2004
+++ edited/arch/sparc64/kernel/sparc64_ksyms.c	Sun Jul  4 13:45:00 2004
@@ -89,7 +89,6 @@
 extern int svr4_setcontext(svr4_ucontext_t *uc, struct pt_regs *regs);
 extern int compat_sys_ioctl(unsigned int fd, unsigned int cmd, u32 arg);
 extern int (*handle_mathemu)(struct pt_regs *, struct fpustate *);
-extern long sparc32_open(const char * filename, int flags, int mode);
 extern int io_remap_page_range(struct vm_area_struct *vma, unsigned long from, unsigned long offset, unsigned long size, pgprot_t prot, int space);
 extern void (*prom_palette)(int);
 
@@ -328,7 +327,6 @@
 EXPORT_SYMBOL(svr4_getcontext);
 EXPORT_SYMBOL(svr4_setcontext);
 EXPORT_SYMBOL(compat_sys_ioctl);
-EXPORT_SYMBOL(sparc32_open);
 EXPORT_SYMBOL(sys_close);
 #endif
 
===== arch/sparc64/kernel/sys32.S 1.9 vs edited =====
--- 1.9/arch/sparc64/kernel/sys32.S	Thu Jun  3 23:51:21 2004
+++ edited/arch/sparc64/kernel/sys32.S	Sun Jul  4 13:45:12 2004
@@ -106,7 +106,7 @@
 SIGN2(sys32_kill, sys_kill, %o0, %o1)
 SIGN1(sys32_nice, sys_nice, %o0)
 SIGN1(sys32_lseek, sys_lseek, %o1)
-SIGN2(sys32_open, sparc32_open, %o1, %o2)
+SIGN2(sys32_open, compat_sys_open, %o1, %o2)
 SIGN1(sys32_readlink, sys_readlink, %o2)
 SIGN1(sys32_sched_get_priority_max, sys_sched_get_priority_max, %o0)
 SIGN1(sys32_sched_get_priority_min, sys_sched_get_priority_min, %o0)
===== arch/sparc64/kernel/sys_sparc32.c 1.102 vs edited =====
--- 1.102/arch/sparc64/kernel/sys_sparc32.c	Wed Jun  2 06:42:33 2004
+++ edited/arch/sparc64/kernel/sys_sparc32.c	Sun Jul  4 13:40:09 2004
@@ -1680,38 +1680,6 @@
 	return ret;
 }
 
-/* This is just a version for 32-bit applications which does
- * not force O_LARGEFILE on.
- */
-
-asmlinkage long sparc32_open(const char __user *filename,
-			     int flags, int mode)
-{
-	char * tmp;
-	int fd, error;
-
-	tmp = getname(filename);
-	fd = PTR_ERR(tmp);
-	if (!IS_ERR(tmp)) {
-		fd = get_unused_fd();
-		if (fd >= 0) {
-			struct file * f = filp_open(tmp, flags, mode);
-			error = PTR_ERR(f);
-			if (IS_ERR(f))
-				goto out_error;
-			fd_install(fd, f);
-		}
-out:
-		putname(tmp);
-	}
-	return fd;
-
-out_error:
-	put_unused_fd(fd);
-	fd = error;
-	goto out;
-}
-
 extern unsigned long do_mremap(unsigned long addr,
 	unsigned long old_len, unsigned long new_len,
 	unsigned long flags, unsigned long new_addr);
===== arch/sparc64/kernel/sys_sunos32.c 1.44 vs edited =====
--- 1.44/arch/sparc64/kernel/sys_sunos32.c	Sat Jun  5 01:39:21 2004
+++ edited/arch/sparc64/kernel/sys_sunos32.c	Sun Jul  4 13:46:41 2004
@@ -1175,13 +1175,11 @@
 	return rval;
 }
 
-extern asmlinkage long sparc32_open(const char * filename, int flags, int mode);
-
 asmlinkage int sunos_open(u32 fname, int flags, int mode)
 {
 	const char *filename = (const char *)(long)fname;
 
-	return sparc32_open(filename, flags, mode);
+	return compat_sys_open(filename, flags, mode);
 }
 
 #define SUNOS_EWOULDBLOCK 35
===== arch/sparc64/solaris/fs.c 1.19 vs edited =====
--- 1.19/arch/sparc64/solaris/fs.c	Tue Sep 23 06:16:30 2003
+++ edited/arch/sparc64/solaris/fs.c	Sun Jul  4 13:47:14 2004
@@ -525,8 +525,6 @@
 	return error;
 }
 
-extern asmlinkage long sparc32_open(const char * filename, int flags, int mode);
-
 asmlinkage int solaris_open(u32 fname, int flags, u32 mode)
 {
 	const char *filename = (const char *)(long)fname;
@@ -542,7 +540,7 @@
 	if (flags & 0x800) fl |= O_NOCTTY;
 	flags = fl;
 
-	return sparc32_open(filename, flags, mode);
+	return compat_sys_open(filename, flags, mode);
 }
 
 #define SOL_F_SETLK	6
===== arch/x86_64/ia32/ia32entry.S 1.37 vs edited =====
--- 1.37/arch/x86_64/ia32/ia32entry.S	Sun Jun 27 08:07:31 2004
+++ edited/arch/x86_64/ia32/ia32entry.S	Sun Jul  4 13:29:25 2004
@@ -310,7 +310,7 @@
 	.quad stub32_fork
 	.quad sys_read
 	.quad sys_write
-	.quad sys32_open		/* 5 */
+	.quad compat_sys_open		/* 5 */
 	.quad sys_close
 	.quad sys32_waitpid
 	.quad sys_creat
===== arch/x86_64/ia32/sys_ia32.c 1.62 vs edited =====
--- 1.62/arch/x86_64/ia32/sys_ia32.c	Sun May 30 21:56:26 2004
+++ edited/arch/x86_64/ia32/sys_ia32.c	Sun Jul  4 13:30:31 2004
@@ -1249,30 +1249,6 @@
 	return ret;
 } 
 
-asmlinkage long sys32_open(const char __user * filename, int flags, int mode)
-{
-	char * tmp;
-	int fd, error;
-
-	/* don't force O_LARGEFILE */
-	tmp = getname(filename);
-	fd = PTR_ERR(tmp);
-	if (!IS_ERR(tmp)) {
-		fd = get_unused_fd();
-		if (fd >= 0) {
-			struct file *f = filp_open(tmp, flags, mode);
-			error = PTR_ERR(f);
-			if (unlikely(IS_ERR(f))) {
-				put_unused_fd(fd); 
-				fd = error;
-			} else
-				fd_install(fd, f);
-		}
-		putname(tmp);
-	}
-	return fd;
-}
-
 struct sigevent32 { 
 	u32 sigev_value;
 	u32 sigev_signo; 
===== fs/compat.c 1.32 vs edited =====
--- 1.32/fs/compat.c	Sun Jun 27 08:07:46 2004
+++ edited/fs/compat.c	Sun Jul  4 13:24:42 2004
@@ -143,6 +143,37 @@
 }
 
 /*
+ * Exactly like fs/open.c:sys_open(), except that it doesn't set the O_LARGEFILE flag.
+ */
+asmlinkage long
+compat_sys_open(const char __user * filename, int flags, int mode)
+{
+	char * tmp;
+	int fd, error;
+
+	tmp = getname(filename);
+	fd = PTR_ERR(tmp);
+	if (!IS_ERR(tmp)) {
+		fd = get_unused_fd();
+		if (fd >= 0) {
+			struct file *f = filp_open(tmp, flags, mode);
+			error = PTR_ERR(f);
+			if (IS_ERR(f))
+				goto out_error;
+			fd_install(fd, f);
+		}
+out:
+		putname(tmp);
+	}
+	return fd;
+
+out_error:
+	put_unused_fd(fd);
+	fd = error;
+	goto out;
+}
+
+/*
  * The following statfs calls are copies of code from fs/open.c and
  * should be checked against those from time to time
  */
===== include/linux/compat.h 1.24 vs edited =====
--- 1.24/include/linux/compat.h	Sat May 29 11:13:12 2004
+++ edited/include/linux/compat.h	Sun Jul  4 13:52:43 2004
@@ -130,5 +130,9 @@
 		compat_ulong_t __user *outp, compat_ulong_t __user *exp,
 		struct compat_timeval __user *tvp);
 
+asmlinkage long compat_sys_open(const char __user * filename,
+				int flags, int mode);
+
+
 #endif /* CONFIG_COMPAT */
 #endif /* _LINUX_COMPAT_H */

Re: move O_LARGEFILE forcing to filp_open()

[William Lee Irwin III]

On Sonntag, 4. Juli 2004 08:41, William Lee Irwin III wrote:
>>                        sys_open() in turn calls filp_open(). So merely
>> moving the forcing of the flag on 64-bit resolves this situation there,
>> though not for 32-bit, whose solution is to appear in the sequel.

On Sun, Jul 04, 2004 at 02:22:56PM +0200, Arnd Bergmann wrote:
> Unfortunately, this will also cause problems for 32-bit emulation, where
> sys32_open currently calls filp_open without forcing O_LARGEFILE for
> 32 bit applications.
> I also noticed that this behavior currently is not implemented on all
> architectures, which in turn would need the patch below.
> Maybe you can find a way to fix both problems.

Your patch is also necessary; thanks for covering these cases.


-- wli

Re: move O_LARGEFILE forcing to filp_open()

[Arnd Bergmann]

[-- Attachment #1: Type: text/plain, Size: 584 bytes --]

On Sonntag, 4. Juli 2004 18:15, William Lee Irwin III wrote:
> Your patch is also necessary; thanks for covering these cases.

I'm not sure if you understood the intention of compat_sys_open
right. Old 32 bit applications assume they are not using O_LARGEFILE,
so you can't switch it on unconditionally in filp_open() for those
cases. With your patch applied, sys_open and compat_sys_open would
be identical again, which reverses the point of my patch.

What is need is a way to turn on O_LARGEFILE on 64 bit archs for
every use of filp_open _except_ from compat_sys_open.

	Arnd <><

[-- Attachment #2: signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: move O_LARGEFILE forcing to filp_open()

[William Lee Irwin III]

On Sonntag, 4. Juli 2004 18:15, William Lee Irwin III wrote:
>> Your patch is also necessary; thanks for covering these cases.

On Sun, Jul 04, 2004 at 07:22:42PM +0200, Arnd Bergmann wrote:
> I'm not sure if you understood the intention of compat_sys_open
> right. Old 32 bit applications assume they are not using O_LARGEFILE,
> so you can't switch it on unconditionally in filp_open() for those
> cases. With your patch applied, sys_open and compat_sys_open would
> be identical again, which reverses the point of my patch.
> What is need is a way to turn on O_LARGEFILE on 64 bit archs for
> every use of filp_open _except_ from compat_sys_open.

Oh, that's easy, just shove the MAX_NON_LFS check into compat_sys_open().


-- wli

Re: move O_LARGEFILE forcing to filp_open()

[William Lee Irwin III]

On Sonntag, 4. Juli 2004 18:15, William Lee Irwin III wrote:
>>> Your patch is also necessary; thanks for covering these cases.

On Sun, Jul 04, 2004 at 07:22:42PM +0200, Arnd Bergmann wrote:
>> I'm not sure if you understood the intention of compat_sys_open
>> right. Old 32 bit applications assume they are not using O_LARGEFILE,
>> so you can't switch it on unconditionally in filp_open() for those
>> cases. With your patch applied, sys_open and compat_sys_open would
>> be identical again, which reverses the point of my patch.
>> What is need is a way to turn on O_LARGEFILE on 64 bit archs for
>> every use of filp_open _except_ from compat_sys_open.

On Sun, Jul 04, 2004 at 10:27:08AM -0700, William Lee Irwin III wrote:
> Oh, that's easy, just shove the MAX_NON_LFS check into compat_sys_open().

BTW, for some reason that's what I thought you were doing in your patch.


-- wli

Re: move O_LARGEFILE forcing to filp_open()

[William Lee Irwin III]

On Sun, Jul 04, 2004 at 07:22:42PM +0200, Arnd Bergmann wrote:
>>> I'm not sure if you understood the intention of compat_sys_open
>>> right. Old 32 bit applications assume they are not using O_LARGEFILE,
>>> so you can't switch it on unconditionally in filp_open() for those
>>> cases. With your patch applied, sys_open and compat_sys_open would
>>> be identical again, which reverses the point of my patch.
>>> What is need is a way to turn on O_LARGEFILE on 64 bit archs for
>>> every use of filp_open _except_ from compat_sys_open.

On Sun, Jul 04, 2004 at 10:27:08AM -0700, William Lee Irwin III wrote:
>> Oh, that's easy, just shove the MAX_NON_LFS check into compat_sys_open().

On Sun, Jul 04, 2004 at 10:27:50AM -0700, William Lee Irwin III wrote:
> BTW, for some reason that's what I thought you were doing in your patch.

How does this look as an implementation of that suggestion, incremental
to your compat_sys_open() consolidation patch?


-- wli

Index: mm5-2.6.7/fs/compat.c
===================================================================
--- mm5-2.6.7.orig/fs/compat.c	2004-07-04 10:29:04.691152200 -0700
+++ mm5-2.6.7/fs/compat.c	2004-07-04 10:34:33.015239352 -0700
@@ -160,6 +160,12 @@
 			error = PTR_ERR(f);
 			if (IS_ERR(f))
 				goto out_error;
+			if (!(filp->f_flags & O_LARGEFILE) &&
+				i_size_read(file->f_dentry->d_inode) > MAX_NON_LFS) {
+				error = -EFBIG;
+				filp_close(filp, current->files);
+				goto out_error;
+			}
 			fd_install(fd, f);
 		}
 out:

Re: move O_LARGEFILE forcing to filp_open()

[Arnd Bergmann]

[-- Attachment #1: Type: text/plain, Size: 345 bytes --]

On Sonntag, 4. Juli 2004 19:38, William Lee Irwin III wrote:
> How does this look as an implementation of that suggestion, incremental
> to your compat_sys_open() consolidation patch?

At first sight, it looks like (filp->f_flags & O_LARGEFILE) will always
be true after calling filp_open, but I don't have time to look closer
today.

	Arnd <><

[-- Attachment #2: signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: move O_LARGEFILE forcing to filp_open()

[William Lee Irwin III]

On Sonntag, 4. Juli 2004 19:38, William Lee Irwin III wrote:
>> How does this look as an implementation of that suggestion, incremental
>> to your compat_sys_open() consolidation patch?

On Sun, Jul 04, 2004 at 07:49:00PM +0200, Arnd Bergmann wrote:
> At first sight, it looks like (filp->f_flags & O_LARGEFILE) will always
> be true after calling filp_open, but I don't have time to look closer
> today.

Quite right.


Index: mm5-2.6.7/fs/compat.c
===================================================================
--- mm5-2.6.7.orig/fs/compat.c	2004-07-04 10:29:04.691152200 -0700
+++ mm5-2.6.7/fs/compat.c	2004-07-04 10:51:05.329384672 -0700
@@ -160,6 +160,12 @@
 			error = PTR_ERR(f);
 			if (IS_ERR(f))
 				goto out_error;
+			if (!(flags & O_LARGEFILE) &&
+				i_size_read(f->f_dentry->d_inode) > MAX_NON_LFS) {
+				error = -EFBIG;
+				filp_close(f, current->files);
+				goto out_error;
+			}
 			fd_install(fd, f);
 		}
 out:

Re: move O_LARGEFILE forcing to filp_open()

[William Lee Irwin III]

On Sun, Jul 04, 2004 at 07:49:00PM +0200, Arnd Bergmann wrote:
>> At first sight, it looks like (filp->f_flags & O_LARGEFILE) will always
>> be true after calling filp_open, but I don't have time to look closer
>> today.

On Sun, Jul 04, 2004 at 10:52:02AM -0700, William Lee Irwin III wrote:
> Quite right.
[...]
> +			if (!(flags & O_LARGEFILE) &&
> +				i_size_read(f->f_dentry->d_inode) > MAX_NON_LFS) {
> +				error = -EFBIG;
> +				filp_close(f, current->files);
> +				goto out_error;
> +			}

With my stuff, the compat_sys_open() patch, and the compat_sys_open()
MAX_NON_LFS patch in place, things pass quick manual backward
compatibility testing (basically, one small app that does open() of a
large file sees the right results with and without O_LARGEFILE where it
didn't before).

-- wli