[git pull] vfs.git part 2

[git pull] vfs.git part 2

[Al Viro]

	Assorted f_pos race fixes, making do_splice_direct() safe to
call with i_mutex on parent, O_TMPFILE support, Jeff's locks.c series,
->d_hash/->d_compare calling conventions changes from Linus, misc stuff
all over the place.  Please, pull from
git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git for-linus

Shortlog:
Al Viro (46):
      pcm_native: switch to fdget()/fdput()
      do_last(): fix missing checks for LAST_BIND case
      lift file_*_write out of do_splice_from()
      lift file_*_write out of do_splice_direct()
      allow build_open_flags() to return an error
      [O_TMPFILE] it's still short a few helpers, but infrastructure should be OK now...
      allow the temp files created by open() to be linked to
      ext3 ->tmpfile() support
      udf: provide ->tmpfile()
      don't call file_pos_write() if vfs_{read,write}{,v}() fails
      ncpfs: don't bother with EBUSY on removal of busy directories
      proc_fill_cache(): kill pointless check
      proc_pid_readdir(): stop wanking with proc_fill_cache() for /proc/self
      proc_fill_cache(): just make instantiate_t return int
      proc_fill_cache(): clean up, get rid of pointless find_inode_number() use
      coda: don't bother with find_inode_number()
      kill find_inode_number()
      more open-coded file_inode() calls
      comedi: quit wanking with FASYNC in ->release()
      fanotify: quit wanking with FASYNC in ->release()
      btrfs: more open-coded file_inode()
      fuse: another open-coded file_inode()
      ecryptfs: switch ecryptfs_decode_and_decrypt_filename() from dentry to sb
      new helper: fixed_size_llseek()
      mtdchar: switch to fixed_size_llseek()
      zorro: switch to fixed_size_llseek()
      bna: switch to fixed_size_llseek()
      eisa_eeprom: switch to fixed_size_llseek()
      vc: switch to fixed_size_llseek()
      fnic: switch to fixed_size_llseek()
      bfa: switch to fixed_size_llseek()
      wlcore: use *ppos, not file->f_pos
      ps3flash: switch to generic_file_llseek_size()
      constify rw_verify_area()
      splice: lift checks from do_splice_from() into callers
      lpfc: switch to fixed_size_llseek()
      isapnp: switch to fixed_size_llseek()
      pci/proc: switch to fixed_size_llseek()
      ubi/cdev: switch to fixed_size_llseek()
      proc_powerpc: switch to fixed_size_llseek()
      tile-srom: switch to fixed_size_llseek()
      cpqphp_sysfs: switch to fixed_size_llseek()
      block_dev: switch to fixed_size_llseek()
      lseek_execute() doesn't need an inode passed to it
      ext4: ->tmpfile() support
      Document ->tmpfile()

Dan Carpenter (1):
      minix: bug widening a binary "not" operation

David Howells (2):
      Replace a bunch of file->dentry->d_inode refs with file_inode()
      SELinux: Institute file_path_has_perm()

Jeff Layton (12):
      locks: drop the unused filp argument to posix_unblock_lock
      cifs: use posix_unblock_lock instead of locks_delete_block
      locks: make generic_add_lease and generic_delete_lease static
      locks: comment cleanups and clarifications
      locks: make "added" in __posix_lock_file a bool
      locks: encapsulate the fl_link list handling
      locks: protect most of the file_lock handling with i_lock
      locks: avoid taking global lock if possible when waking up blocked waiters
      locks: convert fl_link to a hlist_node
      locks: turn the blocked_list into a hashtable
      locks: add a new "lm_owner_key" lock operation
      locks: give the blocked_hash its own spinlock

Jie Liu (1):
      vfs: export lseek_execute() to modules

Linus Torvalds (1):
      Don't pass inode to ->d_hash() and ->d_compare()

Diffstat:
 Documentation/filesystems/Locking               |   35 ++-
 Documentation/filesystems/vfs.txt               |   24 +-
 arch/alpha/include/uapi/asm/fcntl.h             |    1 +
 arch/arc/kernel/troubleshoot.c                  |    2 +-
 arch/parisc/include/uapi/asm/fcntl.h            |    1 +
 arch/powerpc/kernel/proc_powerpc.c              |   20 +--
 arch/sparc/include/uapi/asm/fcntl.h             |    1 +
 drivers/char/ps3flash.c                         |   28 +--
 drivers/char/tile-srom.c                        |   28 +--
 drivers/mtd/mtdchar.c                           |   20 +--
 drivers/mtd/ubi/cdev.c                          |   26 +--
 drivers/net/ethernet/brocade/bna/bnad_debugfs.c |   22 +--
 drivers/net/wireless/ti/wlcore/debugfs.c        |    4 +-
 drivers/parisc/eisa_eeprom.c                    |   15 +-
 drivers/pci/hotplug/cpqphp_sysfs.c              |   22 +--
 drivers/pci/proc.c                              |   23 +--
 drivers/pnp/isapnp/proc.c                       |   22 +--
 drivers/scsi/bfa/bfad_debugfs.c                 |   28 +--
 drivers/scsi/fnic/fnic_debugfs.c                |   16 +--
 drivers/scsi/lpfc/lpfc_debugfs.c                |   18 +--
 drivers/staging/android/logger.c                |    2 +-
 drivers/staging/comedi/comedi_fops.c            |    3 -
 drivers/tty/vt/vc_screen.c                      |   17 +--
 drivers/zorro/proc.c                            |   22 +--
 fs/adfs/dir.c                                   |    6 +-
 fs/affs/namei.c                                 |   26 +--
 fs/afs/flock.c                                  |    7 +-
 fs/aio.c                                        |    2 +
 fs/block_dev.c                                  |   23 +--
 fs/btrfs/file.c                                 |   15 +--
 fs/btrfs/ioctl.c                                |    8 +-
 fs/ceph/file.c                                  |   11 +-
 fs/ceph/locks.c                                 |    2 +-
 fs/ceph/mds_client.c                            |    8 +-
 fs/cifs/cifsfs.c                                |    2 +-
 fs/cifs/dir.c                                   |    9 +-
 fs/cifs/file.c                                  |   15 +-
 fs/coda/dir.c                                   |    8 +-
 fs/dcache.c                                     |   66 ++----
 fs/ecryptfs/crypto.c                            |    5 +-
 fs/ecryptfs/ecryptfs_kernel.h                   |    2 +-
 fs/ecryptfs/file.c                              |    9 +-
 fs/ecryptfs/inode.c                             |    2 +-
 fs/efivarfs/super.c                             |    9 +-
 fs/exec.c                                       |   10 +-
 fs/ext2/namei.c                                 |   24 ++
 fs/ext3/namei.c                                 |   47 ++++-
 fs/ext4/file.c                                  |   24 +--
 fs/ext4/namei.c                                 |   47 ++++-
 fs/fat/namei_msdos.c                            |    6 +-
 fs/fat/namei_vfat.c                             |   12 +-
 fs/file_table.c                                 |    2 +-
 fs/fuse/file.c                                  |    3 +-
 fs/gfs2/dentry.c                                |    3 +-
 fs/gfs2/file.c                                  |    2 +-
 fs/hfs/hfs_fs.h                                 |    7 +-
 fs/hfs/string.c                                 |    6 +-
 fs/hfsplus/hfsplus_fs.h                         |    7 +-
 fs/hfsplus/unicode.c                            |    7 +-
 fs/hpfs/dentry.c                                |    7 +-
 fs/inode.c                                      |    4 +-
 fs/internal.h                                   |    6 +-
 fs/isofs/inode.c                                |   48 ++---
 fs/isofs/namei.c                                |    3 +-
 fs/jfs/namei.c                                  |    7 +-
 fs/lockd/svclock.c                              |   14 +-
 fs/lockd/svcsubs.c                              |   12 +-
 fs/locks.c                                      |  281 ++++++++++++++++-------
 fs/minix/dir.c                                  |    2 +-
 fs/minix/namei.c                                |   13 +
 fs/namei.c                                      |  113 +++++++---
 fs/ncpfs/dir.c                                  |   45 ++--
 fs/ncpfs/inode.c                                |    4 +
 fs/nfs/delegation.c                             |   10 +-
 fs/nfs/nfs4state.c                              |    8 +-
 fs/nfsd/nfs4state.c                             |    8 +-
 fs/notify/fanotify/fanotify_user.c              |    3 -
 fs/ocfs2/file.c                                 |   12 +-
 fs/open.c                                       |   63 +++--
 fs/proc/base.c                                  |  105 ++++-----
 fs/proc/fd.c                                    |   18 +-
 fs/proc/internal.h                              |    2 +-
 fs/proc/namespaces.c                            |   13 +-
 fs/proc/proc_sysctl.c                           |    7 +-
 fs/read_write.c                                 |   65 ++++--
 fs/splice.c                                     |   38 ++--
 fs/sysv/namei.c                                 |    3 +-
 fs/udf/namei.c                                  |   24 ++
 fs/xfs/xfs_file.c                               |    6 +-
 include/linux/dcache.h                          |   11 +-
 include/linux/fs.h                              |   49 +++--
 include/linux/fsnotify.h                        |    8 +-
 include/uapi/asm-generic/fcntl.h                |    4 +
 mm/shmem.c                                      |   37 +++-
 net/sunrpc/auth_gss/svcauth_gss.c               |    4 +-
 security/integrity/ima/ima_main.c               |    2 +-
 security/selinux/hooks.c                        |   24 ++-
 sound/core/pcm_native.c                         |   40 ++--
 98 files changed, 979 insertions(+), 986 deletions(-)

Re: [git pull] vfs.git part 2

[Linus Torvalds]

On Wed, Jul 3, 2013 at 5:29 AM, Al Viro <viro@zeniv.linux.org.uk> wrote:
>         Assorted f_pos race fixes, making do_splice_direct() safe to
> call with i_mutex on parent, O_TMPFILE support, Jeff's locks.c series,
> ->d_hash/->d_compare calling conventions changes from Linus, misc stuff
> all over the place.  Please, pull from
> git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git for-linus

Btw, the new O_TMPFILE support seems to have a fundamental interface
problem: old kernels ignore that flag, and happily implement totally
different semantics than the intended new ones.

So with a *new* kernel, if the filesystem doesn't support O_TMPFILE,
you get an ENOTSUPP error, and you can fall back on whatever old
tmpfile logic you had.

But with an *old* kernel, O_TMPFILE will just silently be ignored as
an unrecognized flag, and things won't work. If you do

  fd = open("dirname", O_CREAT | O_TMPFILE | O_RDWR, 0666);

it may be that it ends up acting as a "create file at specified
directory path" instead of what the user *meant* for it to do, which
was "create unnamed temporary file in the specified directory".

This seems to make the feature actively dangerous. You can't just try
to use it and have a fallback, because that "try to use it" phase may
incorrectly succeed.

Yes, you can force things to not work on old systems by having a slash
at the end of the directory name, but if you ever forget that, you'll
end up with the above problem.

Am I missing something?

              Linus

Re: [git pull] vfs.git part 2

[Al Viro]

On Thu, Jul 11, 2013 at 02:42:54PM -0700, Linus Torvalds wrote:
> On Wed, Jul 3, 2013 at 5:29 AM, Al Viro <viro@zeniv.linux.org.uk> wrote:
> >         Assorted f_pos race fixes, making do_splice_direct() safe to
> > call with i_mutex on parent, O_TMPFILE support, Jeff's locks.c series,
> > ->d_hash/->d_compare calling conventions changes from Linus, misc stuff
> > all over the place.  Please, pull from
> > git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git for-linus
> 
> Btw, the new O_TMPFILE support seems to have a fundamental interface
> problem: old kernels ignore that flag, and happily implement totally
> different semantics than the intended new ones.
> 
> So with a *new* kernel, if the filesystem doesn't support O_TMPFILE,
> you get an ENOTSUPP error, and you can fall back on whatever old
> tmpfile logic you had.
> 
> But with an *old* kernel, O_TMPFILE will just silently be ignored as
> an unrecognized flag, and things won't work. If you do
> 
>   fd = open("dirname", O_CREAT | O_TMPFILE | O_RDWR, 0666);
> 
> it may be that it ends up acting as a "create file at specified
> directory path" instead of what the user *meant* for it to do, which
> was "create unnamed temporary file in the specified directory".
> 
> This seems to make the feature actively dangerous. You can't just try
> to use it and have a fallback, because that "try to use it" phase may
> incorrectly succeed.
> 
> Yes, you can force things to not work on old systems by having a slash
> at the end of the directory name, but if you ever forget that, you'll
> end up with the above problem.
> 
> Am I missing something?

It's slightly less painful than that - if dirname exists, the old kernels
will fail; O_CREAT for existing directory means an error.  So in practice
you can use it safely.  I'm not too happy about that situation, but I
hadn't been able to come up with anything better, short of a new syscall
that would duplicate openat(2), but reject unknown values in flags argument
from the very beginning ;-/  Which is what we probably should've done with
openat(2) itself, but it's too late for that now...

Re: [git pull] vfs.git part 2

[Al Viro]

On Fri, Jul 12, 2013 at 06:48:17AM +0100, Al Viro wrote:
> It's slightly less painful than that - if dirname exists, the old kernels
> will fail; O_CREAT for existing directory means an error.  So in practice
> you can use it safely.  I'm not too happy about that situation, but I
> hadn't been able to come up with anything better, short of a new syscall
> that would duplicate openat(2), but reject unknown values in flags argument
> from the very beginning ;-/  Which is what we probably should've done with
> openat(2) itself, but it's too late for that now...

FWIW, it might make sense to do sys_openat2() that would really validate
the 'flags' argument.  The question is, what checks do we want to do?
The situation for open(2) is complicated by OSF, HPUX and Solaris compat.
AFAICS, we have the following picture:

* two bits are used to encode O_RDONLY/O_WRONLY/O_RDWR/open-for-ioctl
Any combination is allowed.  Ignored with O_PATH.

* O_CLOEXEC - orthogonal to everything else

* O_NOATIME, O_APPEND, O_DIRECT, O_NONBLOCK, O_NDELAY - more or less
arch-independent (O_NDELAY may or may not coincide with O_NONBLOCK,
on parisc O_NOATIME is the same value as O_INVISIBLE, whatever that is).
Can be changed by fcntl(), probably shouldn't be allowed with O_PATH
(right now we just ignore those in that case).  For that matter, F_SETFL
makes no sense for O_PATH descriptors.  O_APPEND makes no sense for O_RDONLY
opens; might be better to leave that as-is, though.

* O_DIRECTORY - shouldn't be allowed with O_TMPFILE.  Requires either O_RDONLY
or O_PATH (in which case both lower bits are ignored anyway).

* O_NOFOLLOW - orthogonal to everything else

* O_NOCTTY, O_LARGEFILE - probably shouldn't be allowed with O_PATH (currently
ignored in that case).

* O_CREAT - makes no sense with O_PATH.  O_TMPFILE currently demands it, but
that's just an attempt to get it fail on old kernels more reliably.

* O_TRUNC - makes no sense with O_PATH.  Pointless with O_TMPFILE.

* O_EXCL - $DEITY-awful mess.  Combination with O_CREAT is probably the
most regular part; however, it is usable without O_CREAT for some
devices, with varying semantics.  I'd mapped "can't link tmpfile in place"
semantics on that one (with O_CREAT | O_TMPFILE).

* O_SYNC and things around it - arch-dependent mess.

* O_BLKSEEK, etc. - weird flags from HPUX et.al.; ignored by the kernel,
OSF fortunately has only O_SYNC/O_DSYNC near that pile and that's the only
one we might realistically care about - Solaris compat is gone and HPUX
one is not much better off.

IMO we should fail on any unknown bits if we go that way, and probably
tighten O_PATH side of things as well.  It's still not too late - we can
just add sys_openat2() and make sys_openat() ignore O_TMPFILE completely.
Do you want to go that way?  If so, let's decide what validation will be
done and spell it out explicitly...

Re: [git pull] vfs.git part 2

[Rasmus Villemoes]

Al Viro <viro@ZenIV.linux.org.uk> writes:

> On Thu, Jul 11, 2013 at 02:42:54PM -0700, Linus Torvalds wrote:
>> But with an *old* kernel, O_TMPFILE will just silently be ignored as
>> an unrecognized flag, and things won't work. If you do
>> 
>>   fd = open("dirname", O_CREAT | O_TMPFILE | O_RDWR, 0666);
>> 
>> it may be that it ends up acting as a "create file at specified
>> directory path" instead of what the user *meant* for it to do, which
>> was "create unnamed temporary file in the specified directory".
>> 
>
> It's slightly less painful than that - if dirname exists, the old kernels
> will fail; O_CREAT for existing directory means an error.

But isn't the problem the case where dirname does not exist? I.e., the
application has to make sure that "/some/where" exists and is a directory
before open("/some/where", O_CREAT | O_TMPFILE | O_RDWR, 0666) can be
relied upon to fail on kernels not recognizing O_TMPFILE, instead of
just creating "where" in "/some".

Just thinking out loud, and please tell me to shut up if it doesn't make
sense: The documentation for O_DIRECTORY seems to imply that one could
require O_DIRECTORY to be given when using O_TMPFILE. The "If pathname
is not a directory, cause the open to fail" certainly seems to make
sense when O_TMPFILE is used, and older kernels should complain when
seeing the O_CREAT|O_DIRECTORY combination. It is a hack, though.

Rasmus

Re: [git pull] vfs.git part 2

[Al Viro]

On Fri, Jul 12, 2013 at 12:02:45PM +0000, Rasmus Villemoes wrote:

> But isn't the problem the case where dirname does not exist? I.e., the
> application has to make sure that "/some/where" exists and is a directory
> before open("/some/where", O_CREAT | O_TMPFILE | O_RDWR, 0666) can be
> relied upon to fail on kernels not recognizing O_TMPFILE, instead of
> just creating "where" in "/some".
> 
> Just thinking out loud, and please tell me to shut up if it doesn't make
> sense: The documentation for O_DIRECTORY seems to imply that one could
> require O_DIRECTORY to be given when using O_TMPFILE. The "If pathname
> is not a directory, cause the open to fail" certainly seems to make
> sense when O_TMPFILE is used, and older kernels should complain when
> seeing the O_CREAT|O_DIRECTORY combination. It is a hack, though.

They should, but they won't ;-/  It's the same problem - we do *not*
validate the flags argument.  We'll get to do_last(), hit lookup_open(),
which will create the sucker and go to finish_open_created.  Which is
past the logics checking for LOOKUP_DIRECTORY trying to return a non-directory
and it would've been too late to fail anyway - the file has already been
created.  IOW, O_DIRECTORY is ignored when O_CREAT is present *and* file
didn't exist already.  In that case we almost certainly can treat that as a
bug (i.e. start failing open() on O_CREAT | O_DIRECTORY in all cases -
I'd be _very_ surprised if somebody called open() with such combination of
flags), but that doesn't help with older kernels...

Re: [git pull] vfs.git part 2

[Rasmus Villemoes]

Al Viro <viro@ZenIV.linux.org.uk> writes:

> On Fri, Jul 12, 2013 at 12:02:45PM +0000, Rasmus Villemoes wrote:
>
>> But isn't the problem the case where dirname does not exist? I.e., the
>> application has to make sure that "/some/where" exists and is a directory
>> before open("/some/where", O_CREAT | O_TMPFILE | O_RDWR, 0666) can be
>> relied upon to fail on kernels not recognizing O_TMPFILE, instead of
>> just creating "where" in "/some".
>> 
>> Just thinking out loud, and please tell me to shut up if it doesn't make
>> sense: The documentation for O_DIRECTORY seems to imply that one could
>> require O_DIRECTORY to be given when using O_TMPFILE. The "If pathname
>> is not a directory, cause the open to fail" certainly seems to make
>> sense when O_TMPFILE is used, and older kernels should complain when
>> seeing the O_CREAT|O_DIRECTORY combination. It is a hack, though.
>
> They should, but they won't ;-/

I see; I should test before I post, but...

> It's the same problem - we do *not* validate the flags argument.
> We'll get to do_last(), hit lookup_open(), which will create the
> sucker and go to finish_open_created.  Which is past the logics
> checking for LOOKUP_DIRECTORY trying to return a non-directory and it
> would've been too late to fail anyway - the file has already been
> created.  IOW, O_DIRECTORY is ignored when O_CREAT is present *and*
> file didn't exist already.  In that case we almost certainly can treat
> that as a bug (i.e. start failing open() on O_CREAT | O_DIRECTORY in
> all cases - I'd be _very_ surprised if somebody called open() with
> such combination of flags), but that doesn't help with older
> kernels...

... it seems that if one then omits O_CREAT, things work out ok, as long
as one uses O_RDWR (which is the only sane thing to do with O_TMPFILE, I
guess):

open("/tmp/test/dir", O_DIRECTORY | O_RDWR, 0666) -> -1; Is a directory
open("/tmp/test/dir", O_DIRECTORY | O_RDONLY, 0666) -> 3; Success
open("/tmp/test/file", O_DIRECTORY | O_RDWR, 0666) -> -1; Not a directory
open("/tmp/test/link_to_file", O_DIRECTORY | O_RDWR, 0666) -> -1; Not a directory
open("/tmp/test/link_to_nowhere", O_DIRECTORY | O_RDWR, 0666) -> -1; No such file or directory
open("/tmp/test/link_to_dir", O_DIRECTORY | O_RDWR, 0666) -> -1; Is a directory
open("/tmp/test/link_to_dir", O_DIRECTORY | O_RDONLY, 0666) -> 3; Success
open("/tmp/test/link_to_dir", O_NOFOLLOW | O_DIRECTORY | O_RDWR, 0666) -> -1; Too many levels of symbolic links
open("/tmp/test/link_to_dir", O_NOFOLLOW | O_DIRECTORY | O_RDONLY, 0666) -> -1; Too many levels of symbolic links

(The above flags are what an old kernel would effectively see with or
without O_TMPFILE present, I suppose.)

How about simply making O_TMPFILE == O_DIRECTORY | O_RDWR |
O_TMPFILE_INTERNAL, and letting the correct use be 

open("/some/dir", O_TMPFILE) [with or without a mode argument]

Using O_DIRECTORY when we don't want to open a directory, and omitting
O_CREAT when we do want to create something new, is somewhat
counter-intuitive, but I think this would solve the problem with old
kernels.

Rasmus

Re: [git pull] vfs.git part 2

[Al Viro]

On Fri, Jul 12, 2013 at 04:30:45PM +0000, Rasmus Villemoes wrote:
> >> Just thinking out loud, and please tell me to shut up if it doesn't make
> >> sense: The documentation for O_DIRECTORY seems to imply that one could
> >> require O_DIRECTORY to be given when using O_TMPFILE. The "If pathname
> >> is not a directory, cause the open to fail" certainly seems to make
> >> sense when O_TMPFILE is used, and older kernels should complain when
> >> seeing the O_CREAT|O_DIRECTORY combination. It is a hack, though.
> >
> > They should, but they won't ;-/
> 
> I see; I should test before I post, but...
> 
> > It's the same problem - we do *not* validate the flags argument.
> > We'll get to do_last(), hit lookup_open(), which will create the
> > sucker and go to finish_open_created.  Which is past the logics
> > checking for LOOKUP_DIRECTORY trying to return a non-directory and it
> > would've been too late to fail anyway - the file has already been
> > created.  IOW, O_DIRECTORY is ignored when O_CREAT is present *and*
> > file didn't exist already.  In that case we almost certainly can treat
> > that as a bug (i.e. start failing open() on O_CREAT | O_DIRECTORY in
> > all cases - I'd be _very_ surprised if somebody called open() with
> > such combination of flags), but that doesn't help with older
> > kernels...
> 
> ... it seems that if one then omits O_CREAT, things work out ok, as long
> as one uses O_RDWR (which is the only sane thing to do with O_TMPFILE, I
> guess):
> 
> open("/tmp/test/dir", O_DIRECTORY | O_RDWR, 0666) -> -1; Is a directory
> open("/tmp/test/dir", O_DIRECTORY | O_RDONLY, 0666) -> 3; Success
> open("/tmp/test/file", O_DIRECTORY | O_RDWR, 0666) -> -1; Not a directory
> open("/tmp/test/link_to_file", O_DIRECTORY | O_RDWR, 0666) -> -1; Not a directory
> open("/tmp/test/link_to_nowhere", O_DIRECTORY | O_RDWR, 0666) -> -1; No such file or directory
> open("/tmp/test/link_to_dir", O_DIRECTORY | O_RDWR, 0666) -> -1; Is a directory
> open("/tmp/test/link_to_dir", O_DIRECTORY | O_RDONLY, 0666) -> 3; Success
> open("/tmp/test/link_to_dir", O_NOFOLLOW | O_DIRECTORY | O_RDWR, 0666) -> -1; Too many levels of symbolic links
> open("/tmp/test/link_to_dir", O_NOFOLLOW | O_DIRECTORY | O_RDONLY, 0666) -> -1; Too many levels of symbolic links
> 
> (The above flags are what an old kernel would effectively see with or
> without O_TMPFILE present, I suppose.)
> 
> How about simply making O_TMPFILE == O_DIRECTORY | O_RDWR |
> O_TMPFILE_INTERNAL, and letting the correct use be 
> 
> open("/some/dir", O_TMPFILE) [with or without a mode argument]
> 
> Using O_DIRECTORY when we don't want to open a directory, and omitting
> O_CREAT when we do want to create something new, is somewhat
> counter-intuitive, but I think this would solve the problem with old
> kernels.

Hrm...  I can't say I like it, but it's almost OK; the only problem here
is the bug fixed by commit bc77daa78 - on some of the old kernels (including
3.10, BTW) we used to allow opening /proc/self/fd/0 with O_DIRECTORY|O_RDWR ;-/

Said that, I think it's more tolerable than the kludge I came up with -
one would need to pass it a procfs symlink as argument to hit that.
Linus, your opinion?

Re: [git pull] vfs.git part 2

[Al Viro]

On Fri, Jul 12, 2013 at 08:13:21PM +0100, Al Viro wrote:

> > Using O_DIRECTORY when we don't want to open a directory, and omitting
> > O_CREAT when we do want to create something new, is somewhat
> > counter-intuitive, but I think this would solve the problem with old
> > kernels.
> 
> Hrm...  I can't say I like it, but it's almost OK; the only problem here
> is the bug fixed by commit bc77daa78 - on some of the old kernels (including
> 3.10, BTW) we used to allow opening /proc/self/fd/0 with O_DIRECTORY|O_RDWR ;-/
> 
> Said that, I think it's more tolerable than the kludge I came up with -
> one would need to pass it a procfs symlink as argument to hit that.
> Linus, your opinion?

I mean something like this:

Safer ABI for O_TMPFILE

[suggested by Rasmus Villemoes] make O_DIRECTORY | O_RDWR part of O_TMPFILE;
that will fail on old kernels in a lot more cases than what I came up with.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
diff --git a/arch/alpha/include/uapi/asm/fcntl.h b/arch/alpha/include/uapi/asm/fcntl.h
index dfdadb0..09f49a6 100644
--- a/arch/alpha/include/uapi/asm/fcntl.h
+++ b/arch/alpha/include/uapi/asm/fcntl.h
@@ -32,7 +32,7 @@
 #define O_SYNC		(__O_SYNC|O_DSYNC)
 
 #define O_PATH		040000000
-#define O_TMPFILE	0100000000
+#define __O_TMPFILE	0100000000
 
 #define F_GETLK		7
 #define F_SETLK		8
diff --git a/arch/parisc/include/uapi/asm/fcntl.h b/arch/parisc/include/uapi/asm/fcntl.h
index cc61c47..34a46cb 100644
--- a/arch/parisc/include/uapi/asm/fcntl.h
+++ b/arch/parisc/include/uapi/asm/fcntl.h
@@ -20,7 +20,7 @@
 #define O_INVISIBLE	004000000 /* invisible I/O, for DMAPI/XDSM */
 
 #define O_PATH		020000000
-#define O_TMPFILE	040000000
+#define __O_TMPFILE	040000000
 
 #define F_GETLK64	8
 #define F_SETLK64	9
diff --git a/arch/sparc/include/uapi/asm/fcntl.h b/arch/sparc/include/uapi/asm/fcntl.h
index d73e5e0..7e8ace5 100644
--- a/arch/sparc/include/uapi/asm/fcntl.h
+++ b/arch/sparc/include/uapi/asm/fcntl.h
@@ -35,7 +35,7 @@
 #define O_SYNC		(__O_SYNC|O_DSYNC)
 
 #define O_PATH		0x1000000
-#define O_TMPFILE	0x2000000
+#define __O_TMPFILE	0x2000000
 
 #define F_GETOWN	5	/*  for sockets. */
 #define F_SETOWN	6	/*  for sockets. */
diff --git a/fs/namei.c b/fs/namei.c
index b2beee7..8b61d10 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2977,7 +2977,7 @@ static struct file *path_openat(int dfd, struct filename *pathname,
 
 	file->f_flags = op->open_flag;
 
-	if (unlikely(file->f_flags & O_TMPFILE)) {
+	if (unlikely(file->f_flags & __O_TMPFILE)) {
 		error = do_tmpfile(dfd, pathname, nd, flags, op, file, &opened);
 		goto out;
 	}
diff --git a/fs/open.c b/fs/open.c
index fca72c4..2a7d4ee 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -840,8 +840,8 @@ static inline int build_open_flags(int flags, umode_t mode, struct open_flags *o
 	if (flags & __O_SYNC)
 		flags |= O_DSYNC;
 
-	if (flags & O_TMPFILE) {
-		if (!(flags & O_CREAT))
+	if (flags & __O_TMPFILE) {
+		if ((flags & O_TMPFILE) != O_TMPFILE)
 			return -EINVAL;
 		acc_mode = MAY_OPEN | ACC_MODE(flags);
 	} else if (flags & O_PATH) {
diff --git a/include/uapi/asm-generic/fcntl.h b/include/uapi/asm-generic/fcntl.h
index 06632be..9b19c49 100644
--- a/include/uapi/asm-generic/fcntl.h
+++ b/include/uapi/asm-generic/fcntl.h
@@ -84,10 +84,13 @@
 #define O_PATH		010000000
 #endif
 
-#ifndef O_TMPFILE
-#define O_TMPFILE	020000000
+#ifndef __O_TMPFILE
+#define __O_TMPFILE	020000000
 #endif
 
+/* a horrid kludge trying to make sure that this will fail on old kernels */
+#define O_TMPFILE (__O_TMPFILE | O_DIRECTORY | O_RDWR)
+
 #ifndef O_NDELAY
 #define O_NDELAY	O_NONBLOCK
 #endif

Re: [git pull] vfs.git part 2

[Linus Torvalds]

On Fri, Jul 12, 2013 at 12:39 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> I mean something like this:
>
> Safer ABI for O_TMPFILE
>
> [suggested by Rasmus Villemoes] make O_DIRECTORY | O_RDWR part of O_TMPFILE;
> that will fail on old kernels in a lot more cases than what I came up with.

So see earlier about why I'm not convinced about O_RDWR. But even if
we really want that (and it might be better to start off too narrow
than accept anything else) your patch tests those bits the wrong way
(any O_RDWR test should be done using the O_ACCMODE mask, not using
the O_RDWR value itself as a mask)

Also, to make sure that the "no preexisting file" case fails, I still
think you should also verify that O_CREAT is not set. Otherwise
Rasmus' case

    open("/tmp/test/link_to_nowhere", O_DIRECTORY | O_RDWR, 0666) ->
-1; No such file or directory

can work, and the case that rasmus didn't have at all (non-existent
pathname) also just silently succeeds by creating a file instead of
the expected directory..

So you could have something like

  #define O_TMPFILE (__O_TMPFILE | O_DIRECTORY | O_RDWR)
  #define O_TMPFILE_MASK (__O_TMPFILE | O_DIRECTORY | O_CREAT | O_ACCMODE)

and then use

   if ((flags & O_TMPFILE_MASK) != O_TMPFILE)
       return -ENOTSUPP;

or whatever.

Hmm?

              Linus

Re: [git pull] vfs.git part 2

[Al Viro]

On Fri, Jul 12, 2013 at 01:21:07PM -0700, Linus Torvalds wrote:
> So you could have something like
> 
>   #define O_TMPFILE (__O_TMPFILE | O_DIRECTORY | O_RDWR)
>   #define O_TMPFILE_MASK (__O_TMPFILE | O_DIRECTORY | O_CREAT | O_ACCMODE)
> 
> and then use
> 
>    if ((flags & O_TMPFILE_MASK) != O_TMPFILE)
>        return -ENOTSUPP;
> 
> or whatever.

Point...  I'd rather use EINVAL, though.  FWIW, we might want to add
openat2() at some later point, with proper validation of arguments; I really
don't want to think of the kludges we'll need the next time we add an
open flag...

Safer ABI for O_TMPFILE

[suggested by Rasmus Villemoes] make O_DIRECTORY | O_RDWR part of O_TMPFILE;
that will fail on old kernels in a lot more cases than what I came up with.
And make sure O_CREAT doesn't get there...

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
diff --git a/arch/alpha/include/uapi/asm/fcntl.h b/arch/alpha/include/uapi/asm/fcntl.h
index dfdadb0..09f49a6 100644
--- a/arch/alpha/include/uapi/asm/fcntl.h
+++ b/arch/alpha/include/uapi/asm/fcntl.h
@@ -32,7 +32,7 @@
 #define O_SYNC		(__O_SYNC|O_DSYNC)
 
 #define O_PATH		040000000
-#define O_TMPFILE	0100000000
+#define __O_TMPFILE	0100000000
 
 #define F_GETLK		7
 #define F_SETLK		8
diff --git a/arch/parisc/include/uapi/asm/fcntl.h b/arch/parisc/include/uapi/asm/fcntl.h
index cc61c47..34a46cb 100644
--- a/arch/parisc/include/uapi/asm/fcntl.h
+++ b/arch/parisc/include/uapi/asm/fcntl.h
@@ -20,7 +20,7 @@
 #define O_INVISIBLE	004000000 /* invisible I/O, for DMAPI/XDSM */
 
 #define O_PATH		020000000
-#define O_TMPFILE	040000000
+#define __O_TMPFILE	040000000
 
 #define F_GETLK64	8
 #define F_SETLK64	9
diff --git a/arch/sparc/include/uapi/asm/fcntl.h b/arch/sparc/include/uapi/asm/fcntl.h
index d73e5e0..7e8ace5 100644
--- a/arch/sparc/include/uapi/asm/fcntl.h
+++ b/arch/sparc/include/uapi/asm/fcntl.h
@@ -35,7 +35,7 @@
 #define O_SYNC		(__O_SYNC|O_DSYNC)
 
 #define O_PATH		0x1000000
-#define O_TMPFILE	0x2000000
+#define __O_TMPFILE	0x2000000
 
 #define F_GETOWN	5	/*  for sockets. */
 #define F_SETOWN	6	/*  for sockets. */
diff --git a/fs/namei.c b/fs/namei.c
index b2beee7..8b61d10 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2977,7 +2977,7 @@ static struct file *path_openat(int dfd, struct filename *pathname,
 
 	file->f_flags = op->open_flag;
 
-	if (unlikely(file->f_flags & O_TMPFILE)) {
+	if (unlikely(file->f_flags & __O_TMPFILE)) {
 		error = do_tmpfile(dfd, pathname, nd, flags, op, file, &opened);
 		goto out;
 	}
diff --git a/fs/open.c b/fs/open.c
index fca72c4..9156cb0 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -840,8 +840,8 @@ static inline int build_open_flags(int flags, umode_t mode, struct open_flags *o
 	if (flags & __O_SYNC)
 		flags |= O_DSYNC;
 
-	if (flags & O_TMPFILE) {
-		if (!(flags & O_CREAT))
+	if (flags & __O_TMPFILE) {
+		if ((flags & O_TMPFILE_MASK) != O_TMPFILE)
 			return -EINVAL;
 		acc_mode = MAY_OPEN | ACC_MODE(flags);
 	} else if (flags & O_PATH) {
diff --git a/include/uapi/asm-generic/fcntl.h b/include/uapi/asm-generic/fcntl.h
index 06632be..05ac354 100644
--- a/include/uapi/asm-generic/fcntl.h
+++ b/include/uapi/asm-generic/fcntl.h
@@ -84,10 +84,14 @@
 #define O_PATH		010000000
 #endif
 
-#ifndef O_TMPFILE
-#define O_TMPFILE	020000000
+#ifndef __O_TMPFILE
+#define __O_TMPFILE	020000000
 #endif
 
+/* a horrid kludge trying to make sure that this will fail on old kernels */
+#define O_TMPFILE (__O_TMPFILE | O_DIRECTORY | O_RDWR)
+#define O_TMPFILE_MASK (__O_TMPFILE | O_DIRECTORY | O_CREAT | O_ACCMODE)      
+
 #ifndef O_NDELAY
 #define O_NDELAY	O_NONBLOCK
 #endif

Re: [git pull] vfs.git part 2

[Rasmus Villemoes]

Linus Torvalds <torvalds@linux-foundation.org> writes:

> On Fri, Jul 12, 2013 at 12:39 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
>>
>> [suggested by Rasmus Villemoes] make O_DIRECTORY | O_RDWR part of O_TMPFILE;
>> that will fail on old kernels in a lot more cases than what I came up with.
>
> So see earlier about why I'm not convinced about O_RDWR. But even if
> we really want that (and it might be better to start off too narrow
> than accept anything else) your patch tests those bits the wrong way
> (any O_RDWR test should be done using the O_ACCMODE mask, not using
> the O_RDWR value itself as a mask)

On further thought, I think I'll retract the suggestion to include
O_RDWR in O_TMPFILE: If that is done, I don't see how one can ever allow
O_WRONLY functionality without breaking the ABI (or introducing
O_TMPFILE_WRONLY). So I suggest O_TMPFILE == O_DIRECTORY|__O_TMPFILE,
and correct use is open(path, O_TMPFILE|O_RDWR, mode).

Also, O_TMPFILE without O_RDWR (or O_WRONLY) should then give -EINVAL. 

Pros:

The access mode is explicit
Easy to allow O_TMPFILE|O_WRONLY in the future (or now?)
Static code checkers complaining about the lack of
O_{RDONLY,RDWR,WRONLY} in open() (?)

Cons:

We catch one fewer case on older kernels, but not one which is likely to
occur in real programs: On old kernels, O_TMPFILE|O_RDONLY, or
equivalently O_TMPFILE, may give a valid file descriptor (in the case
where path does name a directory). However: Anyone writing a program
using O_TMPFILE probably at least tests on a kernel knowing about that,
and so would be given the -EINVAL error, and the documentation can just
spell out that O_TMPFILE is not compatible with O_RDONLY.

Rasmus

Re: [git pull] vfs.git part 2

[Linus Torvalds]

On Fri, Jul 12, 2013 at 12:13 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
> On Fri, Jul 12, 2013 at 04:30:45PM +0000, Rasmus Villemoes wrote:
>>
>> How about simply making O_TMPFILE == O_DIRECTORY | O_RDWR |
>> O_TMPFILE_INTERNAL, and letting the correct use be
>>
>> open("/some/dir", O_TMPFILE) [with or without a mode argument]
>>
> Hrm...  I can't say I like it, but it's almost OK; the only problem here
> is the bug fixed by commit bc77daa78 - on some of the old kernels (including
> 3.10, BTW) we used to allow opening /proc/self/fd/0 with O_DIRECTORY|O_RDWR ;-/
>
> Said that, I think it's more tolerable than the kludge I came up with -
> one would need to pass it a procfs symlink as argument to hit that.
> Linus, your opinion?

I think I like it. Because we really shouldn't rely on "the directory
already exists", since it's actually quite possible that it doesn't.
Sure, things like /tmp and /usr/tmp we can generally rely on, but
mkstemp() and friends are often done using TMPDIR etc, so for a
O_TMPFILE we really shouldn't assume that the directory is some
long-term stable and reliable thing.

My only suggestion is that we *enforce* that O_DIRECTORY is set, and
that O_CREAT is not set (the latter is the reverse of what we do now),
so that we don't get programs that "happen" to work on older kernels
(the /proc bug thing I think we can ignore - at least it makes the
possibility of accidental problems much *much* less).

That said, I'm not sure about O_RDWR. There are ways to possibly turn
an fd into a new path, so I could imagine O_WRONLY being useful
("create a temporary file, fill in the content, do fdatasync, then
atomically make it appear in the filesystem with a linkat() system
call").

I'd actually want to at least bring up again the possible requirement
that the pathname argument to O_TEMPFILE must end in a '/'. It would
be an easy check to add, and then we could actually drop the whole
O_DIRECTORY flag, and O_CREAT becomes a non-issue too. The only
downside of that is that it might be very inconvenient for user mode
(eg if user-mode just wants to use the TMPDIR environment variable
directly), so it might well make for a "better" patch for the kernel,
but be much worse as an ABI issue.

As to the mode argument: we should encourage people to have it, since
the inode *may* become visible afterwards. See above (can you do
linkat() to just turn an fd into a name? I didn't really check, but I
think you can do it as a "link(/proc/sef/fd/..)" thing regardless).

                    Linus

Re: [git pull] vfs.git part 2

[Al Viro]

On Fri, Jul 12, 2013 at 12:42:15PM -0700, Linus Torvalds wrote:

> As to the mode argument: we should encourage people to have it, since
> the inode *may* become visible afterwards. See above (can you do
> linkat() to just turn an fd into a name? I didn't really check, but I
> think you can do it as a "link(/proc/sef/fd/..)" thing regardless).

link() won't do it, linkat() with the right arguments will (link(2)
doesn't follow symlinks on the oldname).  It's a bit more complicated,
though:
	* vfs_link() will refuse to link with link count that went down to 0
(always had, and that's a damn good idea for security reasons)
	* with O_TMPFILE, vfs_link() will act as usual *if* you give open()
O_EXCL as well - files created that way cannot be linked in, ever.  They
stay with link count 0 and they die as soon as they are closed.  Again,
that's the behaviour we'd have with O_CREAT open + unlink.  tmpfile(3) ought
to call it that way.
	* O_TMPFILE _without_ O_EXCL == you may link it in once you've
decided that you've set it up enough.  In that case you are supposed to
open it, write whatever you want to write, fchmod/fchown/fsetxattr/etc.
and use linkat() to put it in place.  Or use vfs_link() if you are doing
that kernel-side.

[git pull] vfs.git part 2

[Al Viro]

	Consolidating sys_wait* and compat counterparts.  Gets rid of
set_fs()/double-copy mess, simplifies the whole thing (lifting the
copyouts to the syscalls means less headache in the part that does
actual work - fewer failure exits, to start with), gets rid of the
overhead of field-by-field __put_user().

The following changes since commit a8c39544a6eb2093c04afd5005b6192bd0e880c6:

  osf_wait4(): fix infoleak (2017-05-21 13:10:07 -0400)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git work.sys_wait

for you to fetch changes up to 92ebce5ac55dba258c608248dddf59eca3f7f514:

  osf_wait4: switch to kernel_wait4() (2017-05-21 13:16:26 -0400)

----------------------------------------------------------------
Al Viro (9):
      move compat wait4 and waitid next to native variants
      wait4(2)/waitid(2): separate copying rusage to userland
      kernel_wait4()/kernel_waitid(): delay copying status to userland
      waitid(2): leave copyout of siginfo to syscall itself
      lift getrusage() from wait_noreap_copyout()
      kill wait_noreap_copyout()
      wait_task_zombie: consolidate info logics
      waitid(): switch copyout of siginfo to unsafe_put_user()
      osf_wait4: switch to kernel_wait4()

 arch/alpha/kernel/osf_sys.c |  53 ++------
 include/linux/resource.h    |   2 +-
 include/linux/sched/task.h  |   2 +
 kernel/compat.c             |  66 ----------
 kernel/exit.c               | 307 +++++++++++++++++++++++---------------------
 kernel/sys.c                |  16 +--
 6 files changed, 187 insertions(+), 259 deletions(-)