linux-next: manual merge of the vfs tree with the overlayfs tree

linux-next: manual merge of the vfs tree with the overlayfs tree

[Stephen Rothwell]

Hi Al,

Today's linux-next merge of the vfs tree got a conflict in:

  fs/overlayfs/dir.c

between commits:

  659f95a46dd0 ("ovl: add ovl_dentry_is_whiteout()")
  ee2e4303d554 ("ovl: opaque cleanup")
  f7cd4e7b2743 ("ovl: clean up kstat usage")

from the overlayfs tree and commit:

  718324db4435 ("overlayfs: stop abusing kstat")

from the vfs tree.

It looks like commits f7cd4e7b2743 and 718324db4435 are essentially the
smae change (with some white space differences).

I fixed it up (I just took bits from each of teh above) and can carry
the fix as necessary. This is now fixed as far as linux-next is
concerned, but any non trivial conflicts should be mentioned to your
upstream maintainer when your tree is submitted for merging.  You may
also want to consider cooperating with the maintainer of the
conflicting tree to minimise any particularly complex conflicts.

Miklos, that overlayfs tree commit f7cd4e7b2743 has no Signed-off-by
from Al as its Author :-(

-- 
Cheers,
Stephen Rothwell

linux-next: manual merge of the vfs tree with the overlayfs tree

[Stephen Rothwell]

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

Hi all,

Today's linux-next merge of the vfs tree got a conflict in:

  fs/overlayfs/file.c

between commit:

  d46b7cd68336 ("ovl: plumb through flush method")

from the overlayfs tree and commit:

  ae7db6c8bc98 ("ovl: remove unneeded ioctls")

from the vfs tree.

I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging.  You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.

-- 
Cheers,
Stephen Rothwell

diff --cc fs/overlayfs/file.c
index 6e454a294046,9bd4167cc7fb..000000000000
--- a/fs/overlayfs/file.c
+++ b/fs/overlayfs/file.c
@@@ -716,11 -590,6 +610,7 @@@ const struct file_operations ovl_file_o
  	.mmap		= ovl_mmap,
  	.fallocate	= ovl_fallocate,
  	.fadvise	= ovl_fadvise,
- 	.unlocked_ioctl	= ovl_ioctl,
 +	.flush		= ovl_flush,
- #ifdef CONFIG_COMPAT
- 	.compat_ioctl	= ovl_compat_ioctl,
- #endif
  	.splice_read    = generic_file_splice_read,
  	.splice_write   = iter_file_splice_write,
  

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: manual merge of the vfs tree with the overlayfs tree

[Al Viro]

On Tue, Jul 10, 2018 at 04:04:55PM +0100, Al Viro wrote:
> First of all, I'm still not at all convinced that this "noaccount" thing is
> sane, especially since path_open() is exported.  But that aside, __get_empty_filp()
> needs to be shot, just for the name and calling conventions alone.
> 
> It gets a bullshit argument (bool account) *AND* does not get the argument it
> does need.  Note that the first thing get_empty_filp() (now __get...) does
> is
>         const struct cred *cred = current_cred();
> followed by
>         f->f_cred = get_cred(cred);
> 
> Now look at path_open().  What happens to the cred argument it gets?  It goes
> to do_dentry_open(), where it gets passed to security_file_open() and not
> used by anything else.  In security_file_open() we have it passed to
> 	ret = call_int_hook(file_open, 0, file, cred);
> and there are three instances of ->file_open() - apparmor, selinux and tomoyo.
> The last one ignores cred entirely; the other two do checks based on it,
> but *all* of them leave file->f_cred as it was.
> 
> This is not a new crap.  It had been inherited from dentry_open(), which got it
> from "CRED: Pass credentials through dentry_open()" back in 2008.  Note that
> 	* among the callers of dentry_open() (20) and vfs_open() (2 more)
> only these
> fs/cachefiles/rdwr.c:913:       file = dentry_open(&path, O_RDWR | O_LARGEFILE, cache->cache_cred);
> security/apparmor/file.c:695:   devnull = dentry_open(&aa_null, O_RDWR, cred);
> security/selinux/hooks.c:2628:  devnull = dentry_open(&selinux_null, O_RDWR, cred);
> get cred != current_cred().  Which helps masking the issue, but makes the
> decision to add that argument (instead of a separate helper) rather dubious.
> 	* overlayfs itself appears to *have* run into the problem, judging
> by
>         old_cred = ovl_override_creds(inode->i_sb);
>         realfile = path_open(&file->f_path, file->f_flags | O_NOATIME,
>                              realinode, current_cred(), false);
>         revert_creds(old_cred);
> in there.
> 
> Folks, if you have to go to that kind of contortions, why not do it right?
> 	* add static __alloc_file(cred), which would get cred pointer (and not
> use current_cred() internally), allocated a file (without bothering with
> nr_files) and returned it
> 	* have alloc_empty_file(cred) that would do the song and dance
> with nr_files (and used __alloc_file() internally).
> 	* use that as a replacement for get_empty_filp() - path_openat() would
> *probably* use current_cred() for argument, alloc_file() definitely would and
> dentry_open() would pass its cred argument.
> 	* in internal.h, static inline alloc_empty_file_noaccount(cred) would
> use __alloc_file() and set FMODE_NOACCOUNT in case of success.
> 	* do_dentry_open() loses the fucking cred argument - it should be in
> file->f_cred.
> 	* vfs_open() goes away - in your branch it's absolutely pointless.
> 	* path_open() loses its 'account' argument - it's always false.
> Uses alloc_empty_file_noaccount() to allocate the sucker.  And for fsck
> sake, pass it the creds you want to use rather than playing that kind of
> games with override/revert.

FWIW, see vfs.git#work.open2 for part of that program.  I have not pulled
the overlayfs stuff in, but doing the rest based at circa e9cf4c40af4c
("fold put_filp() into fput()") is trivial.  Remains to be done out of the
above:
 	* add static __alloc_file(cred), which would be basically
alloc_empty_file() sans nr_files-related parts.  Make alloc_empty_file()
call it for actual allocation.
 	* in internal.h, static inline alloc_empty_file_noaccount(cred) would
use __alloc_file() and set FMODE_NOACCOUNT in case of success.
	* path_open() loses its 'account' argument - it's always false.
Uses alloc_empty_file_noaccount() to allocate the sucker.  And for fsck
sake, pass it the creds you want to use rather than playing that kind of
games with override/revert.
	* (after the overlayfs changes to vfs_open()) vfs_open() goes away,
expanded into callers.

Stuff currently in work.open2:

Preparatory fixes (this cycle fodder):
      drm_mode_create_lease_ioctl(): fix open-coded filp_clone_open()
      cxl_getfile(): fix double-iput() on alloc_file() failures
      ocxlflash_getfile(): fix double-iput() on alloc_file() failures
More preparatory massage:
      make get_empty_filp() to call file_free_rcu() directly
      fold security_file_free() into file_free()
      turn filp_clone_open() into inline wrapper for dentry_open()
      create_pipe_files(): use fput() if allocation of the second file fails
      make sure do_dentry_open() won't return positive as an error
Providing the right ->f_cred:
      pass creds to get_empty_filp(), make sure dentry_open() passes the right creds
      get rid of cred argument of vfs_open() and do_dentry_open()
      security_file_open(): lose cred argument
      ->file_open(): lose cred argument
Mirror the "do we need fput() or do we need put_filp()" in ->f_mode:
      introduce FMODE_OPENED
      fold put_filp() into fput()
At that point we can start simplifying open-related paths, now that
cleanups are a lot more uniform:
      lift fput() on late failures into path_openat()
      now we can fold open_check_o_direct() into do_dentry_open()
      switch all remaining checks for FILE_OPENED to FMODE_OPENED
Half of the reasons for ->atomic_open() 'opened' argument is gone, let's deal with
the rest:
      introduce FMODE_CREATED and switch to it
      IMA: don't propagate opened through the entire thing
      getting rid of 'opened' argument of ->atomic_open() - step 1
      getting rid of 'opened' argument of ->atomic_open() - part 2
      get rid of 'opened' argument of ->atomic_open() - part 3
      get rid of 'opened' in path_openat() and the helpers downstream
      ->atomic_open(): return 0 in all success cases
      document ->atomic_open() changes
      switch atomic_open() and lookup_open() to returning 0 in all success cases
      kill FILE_{CREATED,OPENED}
At that point we've got _much_ simpler ->atomic_open() calling conventions as well
as the code using it.  Next part - alloc_file() calling conventions:
      new wrapper: alloc_file_pseudo()
      __shmem_file_setup(): reorder allocations
      ... and switch shmem_file_setup() to alloc_file_pseudo()
      cxl_getfile(): switch to alloc_file_pseudo()
      ocxlflash_getfile(): switch to alloc_file_pseudo()
      hugetlb_file_setup(): switch to alloc_file_pseudo()
      anon_inode_getfile(): switch to alloc_file_pseudo()
      create_pipe_files(): switch the first allocation to alloc_file_pseudo()
      new helper: alloc_file_clone()
      do_shmat(): grab shp->shm_file earlier, switch to alloc_file_clone()
      make alloc_file() static
      document alloc_file() changes
And cleanups in pathname-resolving loops from better calling conventions
for path_init()/link_path_walk():
      make path_init() unconditionally paired with terminate_walk()
      allow link_path_walk() to take ERR_PTR()
      few more cleanups of link_path_walk() callers

IMO the diffstat is not too bad -

 Documentation/filesystems/Locking     |   2 +-
 Documentation/filesystems/porting     |  20 +++
 Documentation/filesystems/vfs.txt     |  18 +--
 drivers/gpu/drm/drm_lease.c           |  16 +--
 drivers/misc/cxl/api.c                |  21 +---
 drivers/scsi/cxlflash/ocxl_hw.c       |  24 +---
 fs/9p/vfs_inode.c                     |   7 +-
 fs/9p/vfs_inode_dotl.c                |   7 +-
 fs/aio.c                              |  26 +---
 fs/anon_inodes.c                      |  29 +----
 fs/bad_inode.c                        |   2 +-
 fs/binfmt_misc.c                      |   2 +-
 fs/ceph/file.c                        |   7 +-
 fs/ceph/super.h                       |   3 +-
 fs/cifs/cifsfs.h                      |   3 +-
 fs/cifs/dir.c                         |   7 +-
 fs/file_table.c                       |  71 +++++++----
 fs/fuse/dir.c                         |  10 +-
 fs/gfs2/inode.c                       |  32 +++--
 fs/hugetlbfs/inode.c                  |  55 +++------
 fs/internal.h                         |   6 +-
 fs/namei.c                            | 223 +++++++++++++---------------------
 fs/nfs/dir.c                          |  14 ++-
 fs/nfs/nfs4_fs.h                      |   2 +-
 fs/nfs/nfs4proc.c                     |   2 +-
 fs/nfsd/vfs.c                         |   2 +-
 fs/open.c                             |  86 ++++---------
 fs/pipe.c                             |  38 ++----
 include/linux/file.h                  |   8 +-
 include/linux/fs.h                    |  16 +--
 include/linux/ima.h                   |   4 +-
 include/linux/lsm_hooks.h             |   2 +-
 include/linux/security.h              |   5 +-
 ipc/shm.c                             |  39 +++---
 mm/shmem.c                            |  50 ++------
 net/socket.c                          |  27 +---
 security/apparmor/lsm.c               |   4 +-
 security/integrity/ima/ima.h          |   4 +-
 security/integrity/ima/ima_appraise.c |   4 +-
 security/integrity/ima/ima_main.c     |  16 +--
 security/security.c                   |   4 +-
 security/selinux/hooks.c              |   4 +-
 security/smack/smack_lsm.c            |   6 +-
 security/tomoyo/tomoyo.c              |   2 +-
 44 files changed, 360 insertions(+), 570 deletions(-)

Re: linux-next: manual merge of the vfs tree with the overlayfs tree

[Al Viro]

On Tue, Jul 10, 2018 at 10:17:36AM +1000, Stephen Rothwell wrote:
> --- a/fs/open.c
> +++ b/fs/open.c
> @@@ -731,7 -732,6 +721,7 @@@ static int do_dentry_open(struct file *
>   	static const struct file_operations empty_fops = {};
>   	int error;
>   
> - 	WARN_ON(f->f_mode & ~FMODE_NOACCOUNT);
> ++	WARN_ON(f->f_mode & ~ (FMODE_NOACCOUNT | FMODE_CREATED));
>   	f->f_mode |= OPEN_FMODE(f->f_flags) | FMODE_LSEEK |
>   				FMODE_PREAD | FMODE_PWRITE;

That part is sane

>  +/**
>  + * path_open() - Open an inode by a particular name.
>  + * @path: The name of the file.
>  + * @flags: The O_ flags used to open this file.
>  + * @inode: The inode to open.
>  + * @cred: The task's credentials used when opening this file.
>  + *
>  + * Context: Process context.
>  + * Return: A pointer to a struct file or an IS_ERR pointer.  Cannot return NULL.
>  + */
>  +struct file *path_open(const struct path *path, int flags, struct inode *inode,
>  +		       const struct cred *cred, bool account)
>  +{
>  +	struct file *file;
>  +	int error;
>  +
>  +	file = __get_empty_filp(account);
>  +	if (IS_ERR(file))
>  +		return file;
>   
>  +	file->f_flags = flags;
>   	file->f_path = *path;
>  -	return do_dentry_open(file, d_backing_inode(dentry), NULL, cred);
>  +	error = do_dentry_open(file, inode, NULL, cred);
>  +	if (error) {
> - 		put_filp(file);
> ++		if (file->f_mode & FMODE_OPENED)
> ++			fput(file);
> ++		else
> ++			put_filp(file);
>  +		return ERR_PTR(error);
>  +	}
>  +
> - 	error = open_check_o_direct(file);
> - 	if (error) {
> - 		fput(file);
> - 		file = ERR_PTR(error);
> - 	}
> - 
>  +	return file;
>   }
>  +EXPORT_SYMBOL(path_open);

First of all, I'm still not at all convinced that this "noaccount" thing is
sane, especially since path_open() is exported.  But that aside, __get_empty_filp()
needs to be shot, just for the name and calling conventions alone.

It gets a bullshit argument (bool account) *AND* does not get the argument it
does need.  Note that the first thing get_empty_filp() (now __get...) does
is
        const struct cred *cred = current_cred();
followed by
        f->f_cred = get_cred(cred);

Now look at path_open().  What happens to the cred argument it gets?  It goes
to do_dentry_open(), where it gets passed to security_file_open() and not
used by anything else.  In security_file_open() we have it passed to
	ret = call_int_hook(file_open, 0, file, cred);
and there are three instances of ->file_open() - apparmor, selinux and tomoyo.
The last one ignores cred entirely; the other two do checks based on it,
but *all* of them leave file->f_cred as it was.

This is not a new crap.  It had been inherited from dentry_open(), which got it
from "CRED: Pass credentials through dentry_open()" back in 2008.  Note that
	* among the callers of dentry_open() (20) and vfs_open() (2 more)
only these
fs/cachefiles/rdwr.c:913:       file = dentry_open(&path, O_RDWR | O_LARGEFILE, cache->cache_cred);
security/apparmor/file.c:695:   devnull = dentry_open(&aa_null, O_RDWR, cred);
security/selinux/hooks.c:2628:  devnull = dentry_open(&selinux_null, O_RDWR, cred);
get cred != current_cred().  Which helps masking the issue, but makes the
decision to add that argument (instead of a separate helper) rather dubious.
	* overlayfs itself appears to *have* run into the problem, judging
by
        old_cred = ovl_override_creds(inode->i_sb);
        realfile = path_open(&file->f_path, file->f_flags | O_NOATIME,
                             realinode, current_cred(), false);
        revert_creds(old_cred);
in there.

Folks, if you have to go to that kind of contortions, why not do it right?
	* add static __alloc_file(cred), which would get cred pointer (and not
use current_cred() internally), allocated a file (without bothering with
nr_files) and returned it
	* have alloc_empty_file(cred) that would do the song and dance
with nr_files (and used __alloc_file() internally).
	* use that as a replacement for get_empty_filp() - path_openat() would
*probably* use current_cred() for argument, alloc_file() definitely would and
dentry_open() would pass its cred argument.
	* in internal.h, static inline alloc_empty_file_noaccount(cred) would
use __alloc_file() and set FMODE_NOACCOUNT in case of success.
	* do_dentry_open() loses the fucking cred argument - it should be in
file->f_cred.
	* vfs_open() goes away - in your branch it's absolutely pointless.
	* path_open() loses its 'account' argument - it's always false.
Uses alloc_empty_file_noaccount() to allocate the sucker.  And for fsck
sake, pass it the creds you want to use rather than playing that kind of
games with override/revert.

linux-next: manual merge of the vfs tree with the overlayfs tree

[Stephen Rothwell]

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

Hi all,

Today's linux-next merge of the vfs tree got a conflict in:

  include/linux/fs.h

between commit:

  5c299f73cc9e ("vfs: add path_open()")

from the overlayfs tree and commit:

  bfd4fa6990f0 ("turn filp_clone_open() into inline wrapper for dentry_open()")

from the vfs tree.

I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging.  You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.

-- 
Cheers,
Stephen Rothwell

diff --cc include/linux/fs.h
index be8de5925a12,b3a210bfea85..000000000000
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@@ -152,10 -158,10 +158,13 @@@ typedef int (dio_iodone_t)(struct kioc
  #define FMODE_NONOTIFY		((__force fmode_t)0x4000000)
  
  /* File is capable of returning -EAGAIN if I/O will block */
--#define FMODE_NOWAIT	((__force fmode_t)0x8000000)
++#define FMODE_NOWAIT		((__force fmode_t)0x8000000)
+ 
+ /* File represents mount that needs unmounting */
 -#define FMODE_NEED_UNMOUNT     ((__force fmode_t)0x10000000)
++#define FMODE_NEED_UNMOUNT	((__force fmode_t)0x10000000)
 +
 +/* File does not contribute to nr_files count */
- #define FMODE_NOACCOUNT	((__force fmode_t)0x20000000)
++#define FMODE_NOACCOUNT		((__force fmode_t)0x20000000)
  
  /*
   * Flag for rw_copy_check_uvector and compat_rw_copy_check_uvector
@@@ -2419,8 -2445,10 +2442,12 @@@ extern struct file *filp_open(const cha
  extern struct file *file_open_root(struct dentry *, struct vfsmount *,
  				   const char *, int, umode_t);
  extern struct file * dentry_open(const struct path *, int, const struct cred *);
 +extern struct file *path_open(const struct path *, int, struct inode *,
 +			      const struct cred *, bool);
+ static inline struct file *file_clone_open(struct file *file)
+ {
+ 	return dentry_open(&file->f_path, file->f_flags, file->f_cred);
+ }
  extern int filp_close(struct file *, fl_owner_t id);
  
  extern struct filename *getname_flags(const char __user *, int, int *);

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

linux-next: manual merge of the vfs tree with the overlayfs tree

[Stephen Rothwell]

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

Hi all,

Today's linux-next merge of the vfs tree got conflicts in:

  fs/open.c

between commit:

  d4d6f39c507e ("vfs: optionally don't account file in nr_files")
  88498a6bd8d1 ("vfs: simplify dentry_open()")

from the overlayfs tree and commit:

  5f0cc0005d2e ("introduce FMODE_OPENED")
  13fcca01ef86 ("now we can fold open_check_o_direct() into do_dentry_open()")
  7dfb1eeaea37 ("introduce FMODE_CREATED and switch to it")

from the vfs tree.

I *think* I got this right, but please check.  Is there some way that
these overlayfs commits can be integrated into the vfs tree development
(or vice versa).  Also, I would expect to see *some* Reviewed-by or
Acked-by lines in all this work ...

I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging.  You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.

-- 
Cheers,
Stephen Rothwell

diff --cc fs/open.c
index 52a1ea584669,39dcbf13031c..000000000000
--- a/fs/open.c
+++ b/fs/open.c
@@@ -731,7 -732,6 +721,7 @@@ static int do_dentry_open(struct file *
  	static const struct file_operations empty_fops = {};
  	int error;
  
- 	WARN_ON(f->f_mode & ~FMODE_NOACCOUNT);
++	WARN_ON(f->f_mode & ~ (FMODE_NOACCOUNT | FMODE_CREATED));
  	f->f_mode |= OPEN_FMODE(f->f_flags) | FMODE_LSEEK |
  				FMODE_PREAD | FMODE_PWRITE;
  
@@@ -743,8 -743,7 +733,8 @@@
  	f->f_wb_err = filemap_sample_wb_err(f->f_mapping);
  
  	if (unlikely(f->f_flags & O_PATH)) {
 +		WARN_ON(f->f_mode & FMODE_NOACCOUNT);
- 		f->f_mode = FMODE_PATH;
+ 		f->f_mode = FMODE_PATH | FMODE_OPENED;
  		f->f_op = &empty_fops;
  		return 0;
  	}
@@@ -890,47 -893,14 +884,44 @@@ EXPORT_SYMBOL(file_path)
  int vfs_open(const struct path *path, struct file *file,
  	     const struct cred *cred)
  {
 -	struct dentry *dentry = d_real(path->dentry, NULL, file->f_flags, 0);
 +	file->f_path = *path;
 +	return do_dentry_open(file, d_backing_inode(path->dentry), NULL, cred);
 +}
  
 -	if (IS_ERR(dentry))
 -		return PTR_ERR(dentry);
 +/**
 + * path_open() - Open an inode by a particular name.
 + * @path: The name of the file.
 + * @flags: The O_ flags used to open this file.
 + * @inode: The inode to open.
 + * @cred: The task's credentials used when opening this file.
 + *
 + * Context: Process context.
 + * Return: A pointer to a struct file or an IS_ERR pointer.  Cannot return NULL.
 + */
 +struct file *path_open(const struct path *path, int flags, struct inode *inode,
 +		       const struct cred *cred, bool account)
 +{
 +	struct file *file;
 +	int error;
 +
 +	file = __get_empty_filp(account);
 +	if (IS_ERR(file))
 +		return file;
  
 +	file->f_flags = flags;
  	file->f_path = *path;
 -	return do_dentry_open(file, d_backing_inode(dentry), NULL, cred);
 +	error = do_dentry_open(file, inode, NULL, cred);
 +	if (error) {
- 		put_filp(file);
++		if (file->f_mode & FMODE_OPENED)
++			fput(file);
++		else
++			put_filp(file);
 +		return ERR_PTR(error);
 +	}
 +
- 	error = open_check_o_direct(file);
- 	if (error) {
- 		fput(file);
- 		file = ERR_PTR(error);
- 	}
- 
 +	return file;
  }
 +EXPORT_SYMBOL(path_open);
  
  struct file *dentry_open(const struct path *path, int flags,
  			 const struct cred *cred)

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: manual merge of the vfs tree with the overlayfs tree

[Miklos Szeredi]

On Tue, Jun 19, 2018 at 10:40 AM, David Howells <dhowells@redhat.com> wrote:
> Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
>>  /* These sb flags are internal to the kernel */
>>  #define MS_SUBMOUNT     (1<<26)
>> -#define MS_NOREMOTELOCK      (1<<27)
>>  #define MS_NOSEC     (1<<28)
>>  #define MS_BORN              (1<<29)
>>  #define MS_ACTIVE    (1<<30)
>
> Ummm...  Can MS_NOREMOTELOCK be removed?  I know it's listed in the internal
> flags section, but all of these have been exposed to userspace for over a
> year.  Ideally, I'd remove all of these from UAPI, but can anyone guarantee
> that no pieces of userspace refer to them?

The number of potential users of these flags is pretty low, so I think
we can try and remove them and hope nothing breaks.

Probably best left after the dust settles.

Thanks,
Miklos

Re: linux-next: manual merge of the vfs tree with the overlayfs tree

[David Howells]

Stephen Rothwell <sfr@canb.auug.org.au> wrote:

>  /* These sb flags are internal to the kernel */
>  #define MS_SUBMOUNT     (1<<26)
> -#define MS_NOREMOTELOCK	(1<<27)
>  #define MS_NOSEC	(1<<28)
>  #define MS_BORN		(1<<29)
>  #define MS_ACTIVE	(1<<30)

Ummm...  Can MS_NOREMOTELOCK be removed?  I know it's listed in the internal
flags section, but all of these have been exposed to userspace for over a
year.  Ideally, I'd remove all of these from UAPI, but can anyone guarantee
that no pieces of userspace refer to them?

David

linux-next: manual merge of the vfs tree with the overlayfs tree

[Stephen Rothwell]

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

Hi Al,

Today's linux-next merge of the vfs tree got a conflict in:

  include/uapi/linux/fs.h

between commit:

  1d91ca426d8d ("Partially revert "locks: fix file locking on overlayfs"")

from the overlayfs tree and commit:

  28514d1edad4 ("vfs: Suppress MS_* flag defs within the kernel unless explicitly enabled")

from the vfs tree.

I fixed it up (I used the vfs tree version of the file and added the
following merge fix patch) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging.  You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Tue, 19 Jun 2018 11:16:12 +1000
Subject: [PATCH] overlayfs: fix up for MS_ constants moving

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 include/uapi/linux/mount.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/uapi/linux/mount.h b/include/uapi/linux/mount.h
index 3634e065836c..fb104836a51f 100644
--- a/include/uapi/linux/mount.h
+++ b/include/uapi/linux/mount.h
@@ -37,7 +37,6 @@
 
 /* These sb flags are internal to the kernel */
 #define MS_SUBMOUNT     (1<<26)
-#define MS_NOREMOTELOCK	(1<<27)
 #define MS_NOSEC	(1<<28)
 #define MS_BORN		(1<<29)
 #define MS_ACTIVE	(1<<30)
-- 
2.17.1

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

linux-next: manual merge of the vfs tree with the overlayfs tree

[Stephen Rothwell]

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

Hi Al,

Today's linux-next merge of the vfs tree got conflicts in:

  include/linux/fs.h

between commit:

  e8f97b0a52b3 ("vfs: optionally don't account file in nr_files")

from the overlayfs tree and commit:

  db39e40a9682 ("new syscall: open_tree(2)")

from the vfs tree.

I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging.  You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.

-- 
Cheers,
Stephen Rothwell

diff --cc include/linux/fs.h
index ef2a9bba4e11,55ddf123023c..000000000000
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@@ -154,9 -157,9 +157,12 @@@ typedef int (dio_iodone_t)(struct kioc
  /* File is capable of returning -EAGAIN if I/O will block */
  #define FMODE_NOWAIT	((__force fmode_t)0x8000000)
  
+ /* File represents mount that needs unmounting */
+ #define FMODE_NEED_UNMOUNT     ((__force fmode_t)0x10000000)
+ 
 +/* File does not contribute to nr_files count */
- #define FMODE_NOACCOUNT	((__force fmode_t)0x10000000)
++#define FMODE_NOACCOUNT	((__force fmode_t)0x20000000)
 +
  /*
   * Flag for rw_copy_check_uvector and compat_rw_copy_check_uvector
   * that indicates that they should check the contents of the iovec are

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: manual merge of the vfs tree with the overlayfs tree

[Stephen Rothwell]

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

Hi all,

On Tue, 29 May 2018 11:30:35 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> Today's linux-next merge of the vfs tree got a conflict in:
> 
>   fs/read_write.c
> 
> between commit:
> 
>   63ea46a359b2 ("vfs: dedupe: extract helper for a single dedup")
> 
> from the overlayfs tree and commit:
> 
>   227627114799 ("fs: avoid fdput() after failed fdget() in vfs_dedupe_file_range()")
> 
> from the vfs tree.
> 
> I can't see how to easily fix up this conflict, so I effectively dropped
> the vfs tree change.
> 
> I fixed it up (see below) and can carry the fix as necessary. This
> is now fixed as far as linux-next is concerned, but any non trivial
> conflicts should be mentioned to your upstream maintainer when your tree
> is submitted for merging.  You may also want to consider cooperating
> with the maintainer of the conflicting tree to minimise any particularly
> complex conflicts.
> 
> -- 
> Cheers,
> Stephen Rothwell
> 
> diff --cc fs/read_write.c
> index 4d61375a0de4,e83bd9744b5d..000000000000
> --- a/fs/read_write.c
> +++ b/fs/read_write.c
> @@@ -2055,21 -2021,46 +2055,20 @@@ int vfs_dedupe_file_range(struct file *
>   
>   		if (info->reserved) {
>   			info->status = -EINVAL;
>  -		} else if (!(is_admin || (dst_file->f_mode & FMODE_WRITE))) {
>  -			info->status = -EINVAL;
>  -		} else if (file->f_path.mnt != dst_file->f_path.mnt) {
>  -			info->status = -EXDEV;
>  -		} else if (S_ISDIR(dst->i_mode)) {
>  -			info->status = -EISDIR;
>  -		} else if (dst_file->f_op->dedupe_file_range == NULL) {
>  -			info->status = -EINVAL;
>  -		} else {
>  -			deduped = dst_file->f_op->dedupe_file_range(file, off,
>  -							len, dst_file,
>  -							info->dest_offset);
>  -			if (deduped == -EBADE)
>  -				info->status = FILE_DEDUPE_RANGE_DIFFERS;
>  -			else if (deduped < 0)
>  -				info->status = deduped;
>  -			else
>  -				info->bytes_deduped += deduped;
>  +			goto next_loop;
>   		}
>   
>  -next_file:
>  -		mnt_drop_write_file(dst_file);
>  -next_fdput:
>  -		fdput(dst_fd);
>  +		deduped = vfs_dedupe_file_range_one(file, off, dst_file,
>  +						    info->dest_offset, len);
>  +		if (deduped == -EBADE)
>  +			info->status = FILE_DEDUPE_RANGE_DIFFERS;
>  +		else if (deduped < 0)
>  +			info->status = deduped;
>  +		else
>  +			info->bytes_deduped += deduped;
>  +
>   next_loop:
>  +		fdput(dst_fd);
> - 
>   		if (fatal_signal_pending(current))
>   			goto out;
>   	}

This is now a conflict between the overlayfs tree and Linus' tree.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: manual merge of the vfs tree with the overlayfs tree

[Stephen Rothwell]

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

Hi all,

On Tue, 29 May 2018 11:30:35 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> Today's linux-next merge of the vfs tree got a conflict in:
> 
>   fs/read_write.c
> 
> between commit:
> 
>   63ea46a359b2 ("vfs: dedupe: extract helper for a single dedup")
> 
> from the overlayfs tree and commit:
> 
>   227627114799 ("fs: avoid fdput() after failed fdget() in vfs_dedupe_file_range()")
> 
> from the vfs tree.
> 
> I can't see how to easily fix up this conflict, so I effectively dropped
> the vfs tree change.
> 
> I fixed it up (see below) and can carry the fix as necessary. This
> is now fixed as far as linux-next is concerned, but any non trivial
> conflicts should be mentioned to your upstream maintainer when your tree
> is submitted for merging.  You may also want to consider cooperating
> with the maintainer of the conflicting tree to minimise any particularly
> complex conflicts.
> 
> -- 
> Cheers,
> Stephen Rothwell
> 
> diff --cc fs/read_write.c
> index 4d61375a0de4,e83bd9744b5d..000000000000
> --- a/fs/read_write.c
> +++ b/fs/read_write.c
> @@@ -2055,21 -2021,46 +2055,20 @@@ int vfs_dedupe_file_range(struct file *
>   
>   		if (info->reserved) {
>   			info->status = -EINVAL;
>  -		} else if (!(is_admin || (dst_file->f_mode & FMODE_WRITE))) {
>  -			info->status = -EINVAL;
>  -		} else if (file->f_path.mnt != dst_file->f_path.mnt) {
>  -			info->status = -EXDEV;
>  -		} else if (S_ISDIR(dst->i_mode)) {
>  -			info->status = -EISDIR;
>  -		} else if (dst_file->f_op->dedupe_file_range == NULL) {
>  -			info->status = -EINVAL;
>  -		} else {
>  -			deduped = dst_file->f_op->dedupe_file_range(file, off,
>  -							len, dst_file,
>  -							info->dest_offset);
>  -			if (deduped == -EBADE)
>  -				info->status = FILE_DEDUPE_RANGE_DIFFERS;
>  -			else if (deduped < 0)
>  -				info->status = deduped;
>  -			else
>  -				info->bytes_deduped += deduped;
>  +			goto next_loop;
>   		}
>   
>  -next_file:
>  -		mnt_drop_write_file(dst_file);
>  -next_fdput:
>  -		fdput(dst_fd);
>  +		deduped = vfs_dedupe_file_range_one(file, off, dst_file,
>  +						    info->dest_offset, len);
>  +		if (deduped == -EBADE)
>  +			info->status = FILE_DEDUPE_RANGE_DIFFERS;
>  +		else if (deduped < 0)
>  +			info->status = deduped;
>  +		else
>  +			info->bytes_deduped += deduped;
>  +
>   next_loop:
>  +		fdput(dst_fd);
> - 
>   		if (fatal_signal_pending(current))
>   			goto out;
>   	}

This is now a conflict between the overlayfs tree and Linus' tree.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

linux-next: manual merge of the vfs tree with the overlayfs tree

[Stephen Rothwell]

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

Hi all,

Today's linux-next merge of the vfs tree got a conflict in:

  fs/read_write.c

between commit:

  63ea46a359b2 ("vfs: dedupe: extract helper for a single dedup")

from the overlayfs tree and commit:

  227627114799 ("fs: avoid fdput() after failed fdget() in vfs_dedupe_file_range()")

from the vfs tree.

I can't see how to easily fix up this conflict, so I effectively dropped
the vfs tree change.

I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging.  You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.

-- 
Cheers,
Stephen Rothwell

diff --cc fs/read_write.c
index 4d61375a0de4,e83bd9744b5d..000000000000
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@@ -2055,21 -2021,46 +2055,20 @@@ int vfs_dedupe_file_range(struct file *
  
  		if (info->reserved) {
  			info->status = -EINVAL;
 -		} else if (!(is_admin || (dst_file->f_mode & FMODE_WRITE))) {
 -			info->status = -EINVAL;
 -		} else if (file->f_path.mnt != dst_file->f_path.mnt) {
 -			info->status = -EXDEV;
 -		} else if (S_ISDIR(dst->i_mode)) {
 -			info->status = -EISDIR;
 -		} else if (dst_file->f_op->dedupe_file_range == NULL) {
 -			info->status = -EINVAL;
 -		} else {
 -			deduped = dst_file->f_op->dedupe_file_range(file, off,
 -							len, dst_file,
 -							info->dest_offset);
 -			if (deduped == -EBADE)
 -				info->status = FILE_DEDUPE_RANGE_DIFFERS;
 -			else if (deduped < 0)
 -				info->status = deduped;
 -			else
 -				info->bytes_deduped += deduped;
 +			goto next_loop;
  		}
  
 -next_file:
 -		mnt_drop_write_file(dst_file);
 -next_fdput:
 -		fdput(dst_fd);
 +		deduped = vfs_dedupe_file_range_one(file, off, dst_file,
 +						    info->dest_offset, len);
 +		if (deduped == -EBADE)
 +			info->status = FILE_DEDUPE_RANGE_DIFFERS;
 +		else if (deduped < 0)
 +			info->status = deduped;
 +		else
 +			info->bytes_deduped += deduped;
 +
  next_loop:
 +		fdput(dst_fd);
- 
  		if (fatal_signal_pending(current))
  			goto out;
  	}

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: manual merge of the vfs tree with the overlayfs tree

[Stephen Rothwell]

Hi all,

On Thu, 25 Jan 2018 14:31:55 +1100 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> Today's linux-next merge of the vfs tree got a conflict in:
> 
>   fs/dcache.c
> 
> between commit:
> 
>   f9c34674bc60 ("vfs: factor out helpers d_instantiate_anon() and d_alloc_anon()")
> 
> from the overlayfs tree and commit:
> 
>   f1ee616214cb ("VFS: don't keep disconnected dentries on d_anon")
> 
> from the vfs tree.
> 
> I fixed it up (see below) and can carry the fix as necessary. This
> is now fixed as far as linux-next is concerned, but any non trivial
> conflicts should be mentioned to your upstream maintainer when your tree
> is submitted for merging.  You may also want to consider cooperating
> with the maintainer of the conflicting tree to minimise any particularly
> complex conflicts.
> 
> -- 
> Cheers,
> Stephen Rothwell
> 
> diff --cc fs/dcache.c
> index 99bce0ed0213,17e6b84b9656..000000000000
> --- a/fs/dcache.c
> +++ b/fs/dcache.c
> @@@ -1954,16 -1961,18 +1953,18 @@@ static struct dentry *__d_instantiate_a
>   	if (disconnected)
>   		add_flags |= DCACHE_DISCONNECTED;
>   
>  -	spin_lock(&tmp->d_lock);
>  -	__d_set_inode_and_type(tmp, inode, add_flags);
>  -	hlist_add_head(&tmp->d_u.d_alias, &inode->i_dentry);
>  +	spin_lock(&dentry->d_lock);
>  +	__d_set_inode_and_type(dentry, inode, add_flags);
>  +	hlist_add_head(&dentry->d_u.d_alias, &inode->i_dentry);
> - 	hlist_bl_lock(&dentry->d_sb->s_anon);
> - 	hlist_bl_add_head(&dentry->d_hash, &dentry->d_sb->s_anon);
> - 	hlist_bl_unlock(&dentry->d_sb->s_anon);
> + 	if (!disconnected) {
>  -		hlist_bl_lock(&tmp->d_sb->s_roots);
>  -		hlist_bl_add_head(&tmp->d_hash, &tmp->d_sb->s_roots);
>  -		hlist_bl_unlock(&tmp->d_sb->s_roots);
> ++		hlist_bl_lock(&dentry->d_sb->s_roots);
> ++		hlist_bl_add_head(&dentry->d_hash, &tmp->d_sb->s_roots);
> ++		hlist_bl_unlock(&dentry->d_sb->s_roots);
> + 	}
>  -	spin_unlock(&tmp->d_lock);
>  +	spin_unlock(&dentry->d_lock);
>   	spin_unlock(&inode->i_lock);
>   
>  -	return tmp;
>  +	return dentry;
>   
>    out_iput:
>   	iput(inode);

This is now a conflict between the overlayfs tree and Linus' tree.

-- 
Cheers,
Stephen Rothwell

Re: linux-next: manual merge of the vfs tree with the overlayfs tree

[Stephen Rothwell]

Hi all,

On Thu, 25 Jan 2018 14:31:55 +1100 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> + 	if (!disconnected) {
>  -		hlist_bl_lock(&tmp->d_sb->s_roots);
>  -		hlist_bl_add_head(&tmp->d_hash, &tmp->d_sb->s_roots);
>  -		hlist_bl_unlock(&tmp->d_sb->s_roots);
> ++		hlist_bl_lock(&dentry->d_sb->s_roots);
> ++		hlist_bl_add_head(&dentry->d_hash, &tmp->d_sb->s_roots);

I fixed up the "tmp" here ... it should have been "dentry".

-- 
Cheers,
Stephen Rothwell

linux-next: manual merge of the vfs tree with the overlayfs tree

[Stephen Rothwell]

Hi Al,

Today's linux-next merge of the vfs tree got a conflict in:

  fs/dcache.c

between commit:

  f9c34674bc60 ("vfs: factor out helpers d_instantiate_anon() and d_alloc_anon()")

from the overlayfs tree and commit:

  f1ee616214cb ("VFS: don't keep disconnected dentries on d_anon")

from the vfs tree.

I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging.  You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.

-- 
Cheers,
Stephen Rothwell

diff --cc fs/dcache.c
index 99bce0ed0213,17e6b84b9656..000000000000
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@@ -1954,16 -1961,18 +1953,18 @@@ static struct dentry *__d_instantiate_a
  	if (disconnected)
  		add_flags |= DCACHE_DISCONNECTED;
  
 -	spin_lock(&tmp->d_lock);
 -	__d_set_inode_and_type(tmp, inode, add_flags);
 -	hlist_add_head(&tmp->d_u.d_alias, &inode->i_dentry);
 +	spin_lock(&dentry->d_lock);
 +	__d_set_inode_and_type(dentry, inode, add_flags);
 +	hlist_add_head(&dentry->d_u.d_alias, &inode->i_dentry);
- 	hlist_bl_lock(&dentry->d_sb->s_anon);
- 	hlist_bl_add_head(&dentry->d_hash, &dentry->d_sb->s_anon);
- 	hlist_bl_unlock(&dentry->d_sb->s_anon);
+ 	if (!disconnected) {
 -		hlist_bl_lock(&tmp->d_sb->s_roots);
 -		hlist_bl_add_head(&tmp->d_hash, &tmp->d_sb->s_roots);
 -		hlist_bl_unlock(&tmp->d_sb->s_roots);
++		hlist_bl_lock(&dentry->d_sb->s_roots);
++		hlist_bl_add_head(&dentry->d_hash, &tmp->d_sb->s_roots);
++		hlist_bl_unlock(&dentry->d_sb->s_roots);
+ 	}
 -	spin_unlock(&tmp->d_lock);
 +	spin_unlock(&dentry->d_lock);
  	spin_unlock(&inode->i_lock);
  
 -	return tmp;
 +	return dentry;
  
   out_iput:
  	iput(inode);

linux-next: manual merge of the vfs tree with the overlayfs tree

[Stephen Rothwell]

Hi Al,

Today's linux-next merge of the vfs tree got a conflict in:

  fs/overlayfs/super.c

between commits:

  7c84d842e11e ("ovl: reduce the number of arguments for ovl_workdir_create()")
  17d554474412 ("ovl: rename ufs to ofs")

from the overlayfs tree and commit:

  c2c6773f9942 ("VFS: Roll out mount flag differentiation (MS_* -> SB_*) generally")

from the vfs tree.

I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging.  You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.

-- 
Cheers,
Stephen Rothwell

diff --cc fs/overlayfs/super.c
index fd11c0510906,458ba465191a..000000000000
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@@ -324,9 -308,9 +324,9 @@@ static int ovl_show_options(struct seq_
  
  static int ovl_remount(struct super_block *sb, int *flags, char *data)
  {
 -	struct ovl_fs *ufs = sb->s_fs_info;
 +	struct ovl_fs *ofs = sb->s_fs_info;
  
- 	if (!(*flags & MS_RDONLY) && ovl_force_readonly(ofs))
 -	if (!(*flags & SB_RDONLY) && ovl_force_readonly(ufs))
++	if (!(*flags & SB_RDONLY) && ovl_force_readonly(ofs))
  		return -EROFS;
  
  	return 0;
@@@ -1107,117 -954,152 +1107,117 @@@ static struct ovl_entry *ovl_get_lowers
  	sb->s_stack_depth++;
  	if (sb->s_stack_depth > FILESYSTEM_MAX_STACK_DEPTH) {
  		pr_err("overlayfs: maximum fs stacking depth exceeded\n");
 -		goto out_put_lowerpath;
 +		goto out_err;
  	}
  
 -	if (ufs->config.upperdir) {
 -		ufs->upper_mnt = clone_private_mount(&upperpath);
 -		err = PTR_ERR(ufs->upper_mnt);
 -		if (IS_ERR(ufs->upper_mnt)) {
 -			pr_err("overlayfs: failed to clone upperpath\n");
 -			goto out_put_lowerpath;
 -		}
 +	err = ovl_get_lower_layers(ofs, stack, numlower);
 +	if (err)
 +		goto out_err;
  
 -		/* Don't inherit atime flags */
 -		ufs->upper_mnt->mnt_flags &= ~(MNT_NOATIME | MNT_NODIRATIME | MNT_RELATIME);
 +	err = -ENOMEM;
 +	oe = ovl_alloc_entry(numlower);
 +	if (!oe)
 +		goto out_err;
  
 -		sb->s_time_gran = ufs->upper_mnt->mnt_sb->s_time_gran;
 +	for (i = 0; i < numlower; i++) {
 +		oe->lowerstack[i].dentry = dget(stack[i].dentry);
 +		oe->lowerstack[i].layer = &ofs->lower_layers[i];
 +	}
  
 -		ufs->workdir = ovl_workdir_create(sb, ufs, workpath.dentry,
 -						  OVL_WORKDIR_NAME, false);
 -		/*
 -		 * Upper should support d_type, else whiteouts are visible.
 -		 * Given workdir and upper are on same fs, we can do
 -		 * iterate_dir() on workdir. This check requires successful
 -		 * creation of workdir in previous step.
 -		 */
 -		if (ufs->workdir) {
 -			struct dentry *temp;
 -
 -			err = ovl_check_d_type_supported(&workpath);
 -			if (err < 0)
 -				goto out_put_workdir;
 -
 -			/*
 -			 * We allowed this configuration and don't want to
 -			 * break users over kernel upgrade. So warn instead
 -			 * of erroring out.
 -			 */
 -			if (!err)
 -				pr_warn("overlayfs: upper fs needs to support d_type.\n");
 -
 -			/* Check if upper/work fs supports O_TMPFILE */
 -			temp = ovl_do_tmpfile(ufs->workdir, S_IFREG | 0);
 -			ufs->tmpfile = !IS_ERR(temp);
 -			if (ufs->tmpfile)
 -				dput(temp);
 -			else
 -				pr_warn("overlayfs: upper fs does not support tmpfile.\n");
 -
 -			/*
 -			 * Check if upper/work fs supports trusted.overlay.*
 -			 * xattr
 -			 */
 -			err = ovl_do_setxattr(ufs->workdir, OVL_XATTR_OPAQUE,
 -					      "0", 1, 0);
 -			if (err) {
 -				ufs->noxattr = true;
 -				pr_warn("overlayfs: upper fs does not support xattr.\n");
 -			} else {
 -				vfs_removexattr(ufs->workdir, OVL_XATTR_OPAQUE);
 -			}
 +	if (remote)
 +		sb->s_d_op = &ovl_reval_dentry_operations;
 +	else
 +		sb->s_d_op = &ovl_dentry_operations;
  
 -			/* Check if upper/work fs supports file handles */
 -			if (ufs->config.index &&
 -			    !ovl_can_decode_fh(ufs->workdir->d_sb)) {
 -				ufs->config.index = false;
 -				pr_warn("overlayfs: upper fs does not support file handles, falling back to index=off.\n");
 -			}
 -		}
 -	}
 +out:
 +	for (i = 0; i < numlower; i++)
 +		path_put(&stack[i]);
 +	kfree(stack);
 +	kfree(lowertmp);
 +
 +	return oe;
 +
 +out_err:
 +	oe = ERR_PTR(err);
 +	goto out;
 +}
 +
 +static int ovl_fill_super(struct super_block *sb, void *data, int silent)
 +{
 +	struct path upperpath = { };
 +	struct dentry *root_dentry;
 +	struct ovl_entry *oe = NULL;
 +	struct ovl_fs *ofs;
 +	struct cred *cred;
 +	int err;
  
  	err = -ENOMEM;
 -	ufs->lower_mnt = kcalloc(numlower, sizeof(struct vfsmount *), GFP_KERNEL);
 -	if (ufs->lower_mnt == NULL)
 -		goto out_put_workdir;
 -	for (i = 0; i < numlower; i++) {
 -		struct vfsmount *mnt = clone_private_mount(&stack[i]);
 +	ofs = kzalloc(sizeof(struct ovl_fs), GFP_KERNEL);
 +	if (!ofs)
 +		goto out_err;
  
 -		err = PTR_ERR(mnt);
 -		if (IS_ERR(mnt)) {
 -			pr_err("overlayfs: failed to clone lowerpath\n");
 -			goto out_put_lower_mnt;
 -		}
 -		/*
 -		 * Make lower_mnt R/O.  That way fchmod/fchown on lower file
 -		 * will fail instead of modifying lower fs.
 -		 */
 -		mnt->mnt_flags |= MNT_READONLY | MNT_NOATIME;
 +	ofs->creator_cred = cred = prepare_creds();
 +	if (!cred)
 +		goto out_err;
  
 -		ufs->lower_mnt[ufs->numlower] = mnt;
 -		ufs->numlower++;
 +	ofs->config.redirect_dir = ovl_redirect_dir_def;
 +	ofs->config.index = ovl_index_def;
 +	err = ovl_parse_opt((char *) data, &ofs->config);
 +	if (err)
 +		goto out_err;
  
 -		/* Check if all lower layers are on same sb */
 -		if (i == 0)
 -			ufs->same_sb = mnt->mnt_sb;
 -		else if (ufs->same_sb != mnt->mnt_sb)
 -			ufs->same_sb = NULL;
 +	err = -EINVAL;
 +	if (!ofs->config.lowerdir) {
 +		if (!silent)
 +			pr_err("overlayfs: missing 'lowerdir'\n");
 +		goto out_err;
  	}
  
 -	/* If the upper fs is nonexistent, we mark overlayfs r/o too */
 -	if (!ufs->upper_mnt)
 -		sb->s_flags |= SB_RDONLY;
 -	else if (ufs->upper_mnt->mnt_sb != ufs->same_sb)
 -		ufs->same_sb = NULL;
 -
 -	if (!(ovl_force_readonly(ufs)) && ufs->config.index) {
 -		/* Verify lower root is upper root origin */
 -		err = ovl_verify_origin(upperpath.dentry, ufs->lower_mnt[0],
 -					stack[0].dentry, false, true);
 -		if (err) {
 -			pr_err("overlayfs: failed to verify upper root origin\n");
 -			goto out_put_lower_mnt;
 +	sb->s_stack_depth = 0;
 +	sb->s_maxbytes = MAX_LFS_FILESIZE;
 +	if (ofs->config.upperdir) {
 +		if (!ofs->config.workdir) {
 +			pr_err("overlayfs: missing 'workdir'\n");
 +			goto out_err;
  		}
  
 -		ufs->indexdir = ovl_workdir_create(sb, ufs, workpath.dentry,
 -						   OVL_INDEXDIR_NAME, true);
 -		if (ufs->indexdir) {
 -			/* Verify upper root is index dir origin */
 -			err = ovl_verify_origin(ufs->indexdir, ufs->upper_mnt,
 -						upperpath.dentry, true, true);
 -			if (err)
 -				pr_err("overlayfs: failed to verify index dir origin\n");
 +		err = ovl_get_upper(ofs, &upperpath);
 +		if (err)
 +			goto out_err;
  
 -			/* Cleanup bad/stale/orphan index entries */
 -			if (!err)
 -				err = ovl_indexdir_cleanup(ufs->indexdir,
 -							   ufs->upper_mnt,
 -							   stack, numlower);
 -		}
 -		if (err || !ufs->indexdir)
 -			pr_warn("overlayfs: try deleting index dir or mounting with '-o index=off' to disable inodes index.\n");
 +		err = ovl_get_workdir(ofs, &upperpath);
  		if (err)
 -			goto out_put_indexdir;
 +			goto out_err;
 +
 +		if (!ofs->workdir)
- 			sb->s_flags |= MS_RDONLY;
++			sb->s_flags |= SB_RDONLY;
 +
 +		sb->s_stack_depth = ofs->upper_mnt->mnt_sb->s_stack_depth;
 +		sb->s_time_gran = ofs->upper_mnt->mnt_sb->s_time_gran;
 +
  	}
 +	oe = ovl_get_lowerstack(sb, ofs);
 +	if (err)
 +		goto out_err;
  
 -	/* Show index=off/on in /proc/mounts for any of the reasons above */
 -	if (!ufs->indexdir)
 -		ufs->config.index = false;
 +	/* If the upper fs is nonexistent, we mark overlayfs r/o too */
 +	if (!ofs->upper_mnt)
- 		sb->s_flags |= MS_RDONLY;
++		sb->s_flags |= SB_RDONLY;
 +	else if (ofs->upper_mnt->mnt_sb != ofs->same_sb)
 +		ofs->same_sb = NULL;
  
 -	if (remote)
 -		sb->s_d_op = &ovl_reval_dentry_operations;
 -	else
 -		sb->s_d_op = &ovl_dentry_operations;
 +	if (!(ovl_force_readonly(ofs)) && ofs->config.index) {
 +		err = ovl_get_indexdir(ofs, oe, &upperpath);
 +		if (err)
 +			goto out_err;
  
 -	err = -ENOMEM;
 -	ufs->creator_cred = cred = prepare_creds();
 -	if (!cred)
 -		goto out_put_indexdir;
 +		if (!ofs->indexdir)
- 			sb->s_flags |= MS_RDONLY;
++			sb->s_flags |= SB_RDONLY;
 +	}
 +
 +	/* Show index=off/on in /proc/mounts for any of the reasons above */
 +	if (!ofs->indexdir)
 +		ofs->config.index = false;
  
  	/* Never override disk quota limits or use reserved space */
  	cap_lower(cred->cap_effective, CAP_SYS_RESOURCE);
@@@ -1225,8 -1107,13 +1225,8 @@@
  	sb->s_magic = OVERLAYFS_SUPER_MAGIC;
  	sb->s_op = &ovl_super_operations;
  	sb->s_xattr = ovl_xattr_handlers;
 -	sb->s_fs_info = ufs;
 +	sb->s_fs_info = ofs;
- 	sb->s_flags |= MS_POSIXACL | MS_NOREMOTELOCK;
+ 	sb->s_flags |= SB_POSIXACL | SB_NOREMOTELOCK;
  
  	root_dentry = d_make_root(ovl_new_inode(sb, S_IFDIR, 0));
  	if (!root_dentry)

linux-next: manual merge of the vfs tree with the overlayfs tree

[Stephen Rothwell]

Hi Al,

Today's linux-next merge of the vfs tree got a conflict in:

  fs/overlayfs/copy_up.c

between commit:

  4a756233184d ("Revert "ovl: Warn on copy up if a process has a R/O fd open to the lower file"")

from the overlayfs tree and commit:

  450630975da9 ("don't open-code file_inode()")

from the vfs tree.

I fixed it up (the former removed the code updated by the latter) and
can carry the fix as necessary. This is now fixed as far as linux-next
is concerned, but any non trivial conflicts should be mentioned to your
upstream maintainer when your tree is submitted for merging.  You may
also want to consider cooperating with the maintainer of the conflicting
tree to minimise any particularly complex conflicts.

-- 
Cheers,
Stephen Rothwell

linux-next: manual merge of the vfs tree with the overlayfs tree

[Stephen Rothwell]

Hi Al,

Today's linux-next merge of the vfs tree got a conflict in:

  fs/overlayfs/super.c

between commit:

  2b6bc7f48d34 ("ovl: lookup: do getxattr with mounter's permission")

from the overlayfs tree and commit:

  5d6c31910bc0 ("xattr: Add __vfs_{get,set,remove}xattr helpers")

from the vfs tree.

I fixed it up (I just used the overlayfs tree version) and can carry the
fix as necessary. This is now fixed as far as linux-next is concerned,
but any non trivial conflicts should be mentioned to your upstream
maintainer when your tree is submitted for merging.  You may also want
to consider cooperating with the maintainer of the conflicting tree to
minimise any particularly complex conflicts.

-- 
Cheers,
Stephen Rothwell

Re: linux-next: manual merge of the vfs tree with the overlayfs tree

[Miklos Szeredi]

On Mon, Jul 25, 2016 at 2:30 AM, Al Viro <viro@zeniv.linux.org.uk> wrote:
> On Mon, Jul 25, 2016 at 10:24:53AM +1000, Stephen Rothwell wrote:
>> Hi Al,
>>
>> Today's linux-next merge of the vfs tree got a conflict in:
>>
>>   fs/overlayfs/super.c
>>
>> between commit:
>>
>>   e2475b7276d0 ("ovl: check mounter creds on underlying lookup")
>>
>> from the overlayfs tree and commit:
>>
>>   b3ac9a85b31c ("qstr: constify instances in overlayfs")
>>
>> from the vfs tree.
>>
>> I fixed it up (see below) and can carry the fix as necessary. This
>> is now fixed as far as linux-next is concerned, but any non trivial
>> conflicts should be mentioned to your upstream maintainer when your tree
>> is submitted for merging.  You may also want to consider cooperating
>> with the maintainer of the conflicting tree to minimise any particularly
>> complex conflicts.
>
> FWIW, if Miklos could pick that one-liner into overlayfs tree, I'd be only
> happy to drop it from that queue.

OK, picked that one.

Thanks,
Miklos

Re: linux-next: manual merge of the vfs tree with the overlayfs tree

[Al Viro]

On Mon, Jul 25, 2016 at 10:24:53AM +1000, Stephen Rothwell wrote:
> Hi Al,
> 
> Today's linux-next merge of the vfs tree got a conflict in:
> 
>   fs/overlayfs/super.c
> 
> between commit:
> 
>   e2475b7276d0 ("ovl: check mounter creds on underlying lookup")
> 
> from the overlayfs tree and commit:
> 
>   b3ac9a85b31c ("qstr: constify instances in overlayfs")
> 
> from the vfs tree.
> 
> I fixed it up (see below) and can carry the fix as necessary. This
> is now fixed as far as linux-next is concerned, but any non trivial
> conflicts should be mentioned to your upstream maintainer when your tree
> is submitted for merging.  You may also want to consider cooperating
> with the maintainer of the conflicting tree to minimise any particularly
> complex conflicts.

FWIW, if Miklos could pick that one-liner into overlayfs tree, I'd be only
happy to drop it from that queue.

linux-next: manual merge of the vfs tree with the overlayfs tree

[Stephen Rothwell]

Hi Al,

Today's linux-next merge of the vfs tree got a conflict in:

  fs/overlayfs/super.c

between commit:

  e2475b7276d0 ("ovl: check mounter creds on underlying lookup")

from the overlayfs tree and commit:

  b3ac9a85b31c ("qstr: constify instances in overlayfs")

from the vfs tree.

I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging.  You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.

-- 
Cheers,
Stephen Rothwell

diff --cc fs/overlayfs/super.c
index 4c91d9ed4689,bc10c261a006..000000000000
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@@ -419,21 -423,12 +419,21 @@@ static bool ovl_dentry_weird(struct den
  				  DCACHE_OP_COMPARE);
  }
  
 -static inline struct dentry *ovl_lookup_real(struct dentry *dir,
 +static inline struct dentry *ovl_lookup_real(struct super_block *ovl_sb,
 +					     struct dentry *dir,
- 					     struct qstr *name)
+ 					     const struct qstr *name)
  {
 +	const struct cred *old_cred;
  	struct dentry *dentry;
 +	int err;
  
 -	dentry = lookup_hash(name, dir);
 +	old_cred = ovl_override_creds(ovl_sb);
 +	err = inode_permission(dir->d_inode, MAY_EXEC);
 +	if (err)
 +		dentry = ERR_PTR(err);
 +	else
 +		dentry = lookup_hash(name, dir);
 +	revert_creds(old_cred);
  
  	if (IS_ERR(dentry)) {
  		if (PTR_ERR(dentry) == -ENOENT)

linux-next: manual merge of the vfs tree with the overlayfs tree

[Stephen Rothwell]

Hi Al,

Today's linux-next merge of the vfs tree got a conflict in:

  fs/overlayfs/super.c

between commit:

  420598d5bf9c ("ovl: ignore permissions on underlying lookup")

from the overlayfs tree and commit:

  b9e1d435fdf4 ("ovl_lookup_real(): use lookup_one_len_unlocked()")

from the vfs tree.

I fixed it up (I abitrarily chose the overlayfs version (using
lookup_hash() instead of lookup_one_len_unlocked())) and can carry the
fix as necessary. This is now fixed as far as linux-next is concerned,
but any non trivial conflicts should be mentioned to your upstream
maintainer when your tree is submitted for merging.  You may also want
to consider cooperating with the maintainer of the conflicting tree to
minimise any particularly complex conflicts.

-- 
Cheers,
Stephen Rothwell

Re: linux-next: manual merge of the vfs tree with the overlayfs tree

[Miklos Szeredi]

On Mon, May 2, 2016 at 3:08 AM, Al Viro <viro@zeniv.linux.org.uk> wrote:
> On Mon, May 02, 2016 at 10:59:43AM +1000, Stephen Rothwell wrote:
>> Hi Al,
>>
>> Today's linux-next merge of the vfs tree got a conflict in:
>>
>>   fs/overlayfs/super.c
>>
>> between commit:
>>
>>   d478d6a8b8b7 ("ovl: ignore permissions on underlying lookup")
>>
>> from the overlayfs tree and commit:
>>
>>   5cf3e7fecb43 ("ovl_lookup_real(): use lookup_one_len_unlocked()")
>>
>> from the vfs tree.
>>
>> I fixed it up (I used the overlayfs version, since I don't know the
>> locking consequences of teh change from lookup_one_len() to lookup_hash())
>> and can carry the fix as necessary. This is now fixed as far as linux-next
>> is concerned, but any non trivial conflicts should be mentioned to your
>> upstream maintainer when your tree is submitted for merging.  You may
>> also want to consider cooperating with the maintainer of the conflicting
>> tree to minimise any particularly complex conflicts.
>
> Should use lookup_one_len_unlocked(), actually.  lookup_hash() is
> a microoptimization, losing a lot more on excessive i_mutex contention.
> Either variant works, though.

No, here it's not an optimization:

    "More specifically using lookup_one_len() causes a problem when the lower
    directory lacks search permission for a specific user while the upper
    directory does have search permission.  Since lookups are cached, this
    causes inconsistency in behavior: success depends on who did the first
    lookup."

Thanks,
Miklos

Re: linux-next: manual merge of the vfs tree with the overlayfs tree

[Al Viro]

On Mon, May 02, 2016 at 02:08:39AM +0100, Al Viro wrote:

> Should use lookup_one_len_unlocked(), actually.  lookup_hash() is
> a microoptimization, losing a lot more on excessive i_mutex contention.
> Either variant works, though.

PS: if anybody has a better name for lookup_one_len_unlocked(), I'll gladly
rename it; the thing hadn't been in the kernel for too long and the name is
somewhat confusing.  It's an equivalent of
	inode_lock()
	lookup_one_len()
	inode_unlock()
except that it avoids taking the lock when it's not needed.

Re: linux-next: manual merge of the vfs tree with the overlayfs tree

[Al Viro]

On Mon, May 02, 2016 at 10:59:43AM +1000, Stephen Rothwell wrote:
> Hi Al,
> 
> Today's linux-next merge of the vfs tree got a conflict in:
> 
>   fs/overlayfs/super.c
> 
> between commit:
> 
>   d478d6a8b8b7 ("ovl: ignore permissions on underlying lookup")
> 
> from the overlayfs tree and commit:
> 
>   5cf3e7fecb43 ("ovl_lookup_real(): use lookup_one_len_unlocked()")
> 
> from the vfs tree.
> 
> I fixed it up (I used the overlayfs version, since I don't know the
> locking consequences of teh change from lookup_one_len() to lookup_hash())
> and can carry the fix as necessary. This is now fixed as far as linux-next
> is concerned, but any non trivial conflicts should be mentioned to your
> upstream maintainer when your tree is submitted for merging.  You may
> also want to consider cooperating with the maintainer of the conflicting
> tree to minimise any particularly complex conflicts.

Should use lookup_one_len_unlocked(), actually.  lookup_hash() is
a microoptimization, losing a lot more on excessive i_mutex contention.
Either variant works, though.

linux-next: manual merge of the vfs tree with the overlayfs tree

[Stephen Rothwell]

Hi Al,

Today's linux-next merge of the vfs tree got a conflict in:

  fs/overlayfs/super.c

between commit:

  d478d6a8b8b7 ("ovl: ignore permissions on underlying lookup")

from the overlayfs tree and commit:

  5cf3e7fecb43 ("ovl_lookup_real(): use lookup_one_len_unlocked()")

from the vfs tree.

I fixed it up (I used the overlayfs version, since I don't know the
locking consequences of teh change from lookup_one_len() to lookup_hash())
and can carry the fix as necessary. This is now fixed as far as linux-next
is concerned, but any non trivial conflicts should be mentioned to your
upstream maintainer when your tree is submitted for merging.  You may
also want to consider cooperating with the maintainer of the conflicting
tree to minimise any particularly complex conflicts.

-- 
Cheers,
Stephen Rothwell