move O_LARGEFILE forcing to filp_open()
Subsystems (6)
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
fs/open.c +2 -3
@@ -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()
Quoted 6 lines · 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 entrypointfilp_open(), notsys_open().sys_open()in turn callsfilp_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
mm/swapfile.c +2 -2
@@ -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()
Quoted 5 lines · William Lee Irwin III
For 32-bit, one quickly discovers that
swapon()is not given an fd already opened withO_LARGEFILEto act upon and the forcing ofO_LARGEFILEfor 64-bit is irrelevant, as the system call's argument is a path. So this patch manually forces it forswapon()andswapoff().
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()
Quoted 4 lines · William Lee Irwin III
Quoted 4 lines
For 32-bit, one quickly discovers that
swapon()is not given an fd already opened withO_LARGEFILEto act upon and the forcing ofO_LARGEFILEfor 64-bit is irrelevant, as the system call's argument is a path. So this patch manually forces it forswapon()andswapoff().
Quoted 5 lines · Hugh Dickins
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 usedO_LARGEFILEhere.
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
Quoted 3 lines · William Lee Irwin III
sys_open()in turn callsfilp_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 =====
arch/ia64/ia32/ia32_entry.S +1 -1
@@ -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 =====
arch/ia64/ia32/sys_ia32.c +0 -31
@@ -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 =====
arch/mips/kernel/scall64-o32.S +1 -1
@@ -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 =====
arch/parisc/kernel/syscall_table.S +1 -1
@@ -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 =====
arch/ppc64/kernel/misc.S +1 -1
@@ -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 =====
arch/ppc64/kernel/sys_ppc32.c +0 -31
@@ -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 =====
arch/s390/kernel/compat_wrapper.S +1 -1
@@ -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 =====
arch/sparc64/kernel/sparc64_ksyms.c +0 -2
@@ -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 =====
arch/sparc64/kernel/sys32.S +1 -1
@@ -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 =====
arch/sparc64/kernel/sys_sparc32.c +0 -32
@@ -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 =====
arch/sparc64/kernel/sys_sunos32.c +1 -3
@@ -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 =====
arch/sparc64/solaris/fs.c +1 -3
@@ -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 =====
arch/x86_64/ia32/ia32entry.S +1 -1
@@ -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 =====
arch/x86_64/ia32/sys_ia32.c +0 -24
@@ -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 =====
fs/compat.c +31 -0
@@ -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 =====
include/linux/compat.h +4 -0
@@ -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 */
Quoted 3 lines · William Lee Irwin III
Quoted 3 lines
sys_open()in turn callsfilp_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.
Quoted 6 lines · Arnd Bergmann
Unfortunately, this will also cause problems for 32-bit emulation, where
sys32_opencurrently callsfilp_openwithout forcingO_LARGEFILEfor 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
Quoted 1 line · William Lee Irwin III
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 <><
Quoted 1 line · William Lee Irwin III
Quoted 1 line
Your patch is also necessary; thanks for covering these cases.
Quoted 7 lines · Arnd Bergmann
I'm not sure if you understood the intention of
compat_sys_openright. Old 32 bit applications assume they are not usingO_LARGEFILE, so you can't switch it on unconditionally infilp_open()for those cases. With your patch applied,sys_openandcompat_sys_openwould be identical again, which reverses the point of my patch. What is need is a way to turn onO_LARGEFILEon 64 bit archs for every use offilp_open_except_ fromcompat_sys_open.
Oh, that's easy, just shove the MAX_NON_LFS check into compat_sys_open().
-- wli
Quoted 1 line · William Lee Irwin III
Quoted 1 line
Quoted 1 line
Your patch is also necessary; thanks for covering these cases.
Quoted 7 lines · Arnd Bergmann
Quoted 7 lines
I'm not sure if you understood the intention of
compat_sys_openright. Old 32 bit applications assume they are not usingO_LARGEFILE, so you can't switch it on unconditionally infilp_open()for those cases. With your patch applied,sys_openandcompat_sys_openwould be identical again, which reverses the point of my patch. What is need is a way to turn onO_LARGEFILEon 64 bit archs for every use offilp_open_except_ fromcompat_sys_open.
Quoted 1 line · William Lee Irwin III
Oh, that's easy, just shove the
MAX_NON_LFScheck intocompat_sys_open().
BTW, for some reason that's what I thought you were doing in your patch.
-- wli
Quoted 7 lines · Arnd Bergmann
Quoted 7 lines
Quoted 7 lines
I'm not sure if you understood the intention of
compat_sys_openright. Old 32 bit applications assume they are not usingO_LARGEFILE, so you can't switch it on unconditionally infilp_open()for those cases. With your patch applied,sys_openandcompat_sys_openwould be identical again, which reverses the point of my patch. What is need is a way to turn onO_LARGEFILEon 64 bit archs for every use offilp_open_except_ fromcompat_sys_open.
Quoted 1 line · William Lee Irwin III
Quoted 1 line
Oh, that's easy, just shove the
MAX_NON_LFScheck intocompat_sys_open().
Quoted 1 line · William Lee Irwin III
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
fs/compat.c +6 -0
@@ -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:
Quoted 2 lines · William Lee Irwin III
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 <><
Quoted 2 lines · William Lee Irwin III
Quoted 2 lines
How does this look as an implementation of that suggestion, incremental to your
compat_sys_open()consolidation patch?
Quoted 3 lines · Arnd Bergmann
At first sight, it looks like (
filp->f_flags&O_LARGEFILE) will always be true after callingfilp_open, but I don't have time to look closer today.
Quite right.
fs/compat.c +6 -0
@@ -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:
Quoted 3 lines · Arnd Bergmann
Quoted 3 lines
At first sight, it looks like (
filp->f_flags&O_LARGEFILE) will always be true after callingfilp_open, but I don't have time to look closer today.
Quoted 1 line · William Lee Irwin III
Quite right.
[...]
fs/compat.c line 168
Quoted 6 lines
fs/compat.c +6
-0+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