Hello again,
   I've taken the code from fuse/dir.c and modified it slightly to provide a
   pretty minimal workaround, by creating the file with read/write permission
   first and then revoking that permission in a separate call to aufs_setattr
   if necessary. That's not perfect behavior (there is a window during which
   the extended preliminary permissions are visible), but it appears to work.
   I'm not really sure we can do better without using the underlying file
   system's ->atomic_open op, if it has one, so we might have to keep this code
   around for the other file systems.
   diff        -ur        -X       linux-4.0.0-rc7/Documentation/dontdiff
   linux-4.0.0-rc7-aufs/fs/aufs/i_op.c
   linux-4.0.0-rc7-aufs-hacked/fs/aufs/i_op.c
   --- linux-4.0.0-rc7-aufs/fs/aufs/i_op.c    2015-04-04 05:30:05.935306135
   +0000
   +++     linux-4.0.0-rc7-aufs-hacked/fs/aufs/i_op.c       2015-04-07
   15:24:30.521005453 +0000
   @@ -1219,6 +1219,61 @@
   Â
   Â /*----------------------------------------------------------------------
   */
   Â
   +int aufs_atomic_open(struct inode *dir_inode, struct dentry *dentry,
   +Â Â Â  Â Â Â  Â Â Â Â  struct file *file, unsigned open_flags,
   +Â Â Â  Â Â Â  Â Â Â Â  umode_t mode, int *opened)
   +{
   +Â Â Â  struct dentry *res = NULL;
   +Â Â Â  int err;
   +Â Â Â  umode_t prel_mode = mode;
   +Â Â Â
   +Â Â Â  BUG_ON(dentry->d_inode);
   +
   +Â Â Â  if (d_unhashed(dentry)) {
   +Â Â Â  Â Â Â  res = aufs_lookup(dir_inode, dentry, 0);
   +
   +Â Â Â  Â Â Â  if (IS_ERR(res))
   +Â Â Â  Â Â Â  Â Â Â  return PTR_ERR(res);
   +Â Â Â
   +Â Â Â  Â Â Â  if (res)
   +Â Â Â  Â Â Â  Â Â Â  dentry = res;
   +Â Â Â  }
   +
   +Â Â Â  if (!(open_flags & O_CREAT) || dentry->d_inode)
   +Â Â Â  Â Â Â  goto no_open;
   +
   +Â Â Â  if (open_flags & O_RDWR)
   +Â Â Â  Â Â Â  prel_mode |= 0600;
   +Â Â Â  else if (open_flags & O_WRONLY)
   +Â Â Â  Â Â Â  prel_mode |= 0200;
   +Â Â Â  else
   +Â Â Â  Â Â Â  prel_mode |= 0400;
   +Â Â Â
   +Â Â Â  *opened |= FILE_CREATED;
   +
   +Â Â Â   err = aufs_create(dir_inode, dentry, prel_mode, (open_flags &
   O_EXCL) != 0);
   +Â Â Â  if (!err)
   +Â Â Â  Â Â Â  err = finish_open(file, dentry, NULL, opened);
   +Â Â Â  if (!err && prel_mode != mode) {
   +Â Â Â  Â Â Â  struct iattr ia;
   +Â Â Â  Â Â Â  ia.ia_valid = ATTR_MODE;
   +Â Â Â  Â Â Â  ia.ia_mode = mode;
   +Â Â Â  Â Â Â  mutex_lock_nested(&dentry->d_inode->i_mutex, AuLsc_I_PARENT);
   +Â Â Â  Â Â Â  err = notify_change(dentry, &ia, NULL);
   +Â Â Â  Â Â Â  mutex_unlock(&dentry->d_inode->i_mutex);
   +
   +Â Â Â  Â Â Â  if (err)
   +Â Â Â  Â Â Â  Â Â Â  fput(file);
   +Â Â Â  }
   +Â Â Â  dput(res);
   +Â Â Â  return err;
   +
   + no_open:
   +Â Â Â  return finish_no_open(file, res);
   +}
   +
   +/*----------------------------------------------------------------------
   */
   +
   Â struct inode_operations aufs_symlink_iop = {
        .permission    = aufs_permission,
   Â #ifdef CONFIG_FS_POSIX_ACL
   @@ -1271,7 +1326,7 @@
   Â #endif
   Â
        .update_time    = aufs_update_time,
   -Â Â Â  /* no support for atomic_open() */
   +    .atomic_open    = aufs_atomic_open,
   Â
        .tmpfile    = aufs_tmpfile
   Â };
   On Tue, Apr 7, 2015 at 2:45 AM, Pip Cet <[1]pip...@gmail.com> wrote:

   Hi,
   thank you for your response!
   On Mon, Apr 6, 2015 at 6:30 AM, <[2]sf...@users.sourceforge.net> wrote:

     Hello Pip,
     Pip Cet:
     > I've had a look at the relevant code in fs/namei.c, and it appears to me
     > the problem is that lookup_open(), in the absence of an ->atomic_open
     > method, falls back to creating a file with vfs_create, then opening it
     with
     > vfs_open, which fails in this case. It's not clear to me what a good fix
     > would be, though implementing ->atomic_open should work.
     You analized very well.
     I've tried implementing aufs atomic_open several years before, but it
     was complicated and I decided that aufs doesn't support atomic_open.

   I must admit I'm still not sure to what extent this is a bug in aufs, nfs4,
   or the vfs layer.
   Â

     It was not a big issue until now and I have not received such mail.
     Something might changed recently?
     - nfs4 becomes popular
     - vfs or nfs4 internal implementation changed
     No, the changes in outer world should not be related to this
     problem. You reported other versions of aufs failed similarly. And I
     guess aufs have never succeeded open(O_WRONLY|O_CREAT, 0400) on nfs4
     branch.
     Probably it is just a matter of fact that who report first.

   I think it's best to fix it by implementing aufs_atomic_open. Maybe the code
   in fs/fuse/dir.c:fuse_atomic_open can serve as a starting point. However, I
   also think the VFS code and the nfs4 code are, at least, less than optimal
   in not dealing with this case very well.
   The  nfs4  implementation  expects  that  all  file creates go through
   .atomic_open rather than being split by the VFS; that's currently true, but
   I'm not sure it's a good design decision to rely on its being true in future
   versions  of  the  VFS.  The  VFS  layer  could  reasonably  split  an
   open(O_CREAT|O_WRONLY, 0400) into the sequence
   open("file", O_CREAT|O_WRONLY, 0600);
   open("file", O_WRONLY);
   chmod("file", 0400);
   which I believe should be equivalent.
   Â

     > As an experiment, I've disabled nfs4's ->atomic_open method, and the
     result
     > was that with the modified kernel, my test program fails even directly
     on
     > the nfs file system. It still succeeds both directly on a ramfs and on
     an
     > aufs using said ramfs, but that appears to be because ramfs has no
     ->open
     > at all
     Right.
     I've confirmed nfs fh_verify() rejects open(O_WRONLY) for the already
     created readonly file.
     I think aufs should try supporting atomic_open (while it is complicated)
     now. As a first step, would you try this patch? It is just to confirm
     the scenario and prints a [3]kern.info log where aufs should call
     ->atomic_open().

   That message is printed both for tar and for the test program.
   For tar:
   openat(AT_FDCWD,"x",O_WRONLY|O_CREAT|O_EXCL|O_NOCTTY|O_NONBLOCK|O_CLOEXEC,
   0400[Â  416.703027] aufs aufs_lookup:245:tar[2032]: call nfs_atomic_open for
   b0 sivtar/x
   ) = -1 EACCES (Permission denied)
   for test3:
   open("x",     O_WRONLY|O_CREAT,     0400<3>[Â      497.633130]    aufs
   aufs_lookup:245:test3[2065]: call nfs_atomic_open for b0 sivtar/x
   )Â Â Â Â Â Â  = -1 EACCES (Permission denied)
   (sivtar is the name of the directory I ran the tests in, and test3 is the
   name of the test program whose source code I posted).
   Â

     I'd ask you to try verious patterns. By this patch aufs will prints a
     msg. In other words, when you see an error without the aufs log msg, it
     means this first step is not good.

   So far, that hasn't happened.
   Thanks again,
   Pip

References

   1. mailto:pip...@gmail.com
   2. mailto:sf...@users.sourceforge.net
   3. http://kern.info/
diff -ur -X linux-4.0.0-rc7/Documentation/dontdiff 
linux-4.0.0-rc7-aufs/fs/aufs/i_op.c linux-4.0.0-rc7-aufs-hacked/fs/aufs/i_op.c
--- linux-4.0.0-rc7-aufs/fs/aufs/i_op.c 2015-04-04 05:30:05.935306135 +0000
+++ linux-4.0.0-rc7-aufs-hacked/fs/aufs/i_op.c  2015-04-07 15:24:30.521005453 
+0000
@@ -1219,6 +1219,61 @@
 
 /* ---------------------------------------------------------------------- */
 
+int aufs_atomic_open(struct inode *dir_inode, struct dentry *dentry,
+                    struct file *file, unsigned open_flags,
+                    umode_t mode, int *opened)
+{
+       struct dentry *res = NULL;
+       int err;
+       umode_t prel_mode = mode;
+       
+       BUG_ON(dentry->d_inode);
+
+       if (d_unhashed(dentry)) {
+               res = aufs_lookup(dir_inode, dentry, 0);
+
+               if (IS_ERR(res))
+                       return PTR_ERR(res);
+       
+               if (res)
+                       dentry = res;
+       }
+
+       if (!(open_flags & O_CREAT) || dentry->d_inode)
+               goto no_open;
+
+       if (open_flags & O_RDWR)
+               prel_mode |= 0600;
+       else if (open_flags & O_WRONLY)
+               prel_mode |= 0200;
+       else
+               prel_mode |= 0400;
+       
+       *opened |= FILE_CREATED;
+
+       err = aufs_create(dir_inode, dentry, prel_mode, (open_flags & O_EXCL) 
!= 0);
+       if (!err)
+               err = finish_open(file, dentry, NULL, opened);
+       if (!err && prel_mode != mode) {
+               struct iattr ia;
+               ia.ia_valid = ATTR_MODE;
+               ia.ia_mode = mode;
+               mutex_lock_nested(&dentry->d_inode->i_mutex, AuLsc_I_PARENT);
+               err = notify_change(dentry, &ia, NULL);
+               mutex_unlock(&dentry->d_inode->i_mutex);
+
+               if (err)
+                       fput(file);
+       }
+       dput(res);
+       return err;
+
+ no_open:
+       return finish_no_open(file, res);
+}
+
+/* ---------------------------------------------------------------------- */
+
 struct inode_operations aufs_symlink_iop = {
        .permission     = aufs_permission,
 #ifdef CONFIG_FS_POSIX_ACL
@@ -1271,7 +1326,7 @@
 #endif
 
        .update_time    = aufs_update_time,
-       /* no support for atomic_open() */
+       .atomic_open    = aufs_atomic_open,
 
        .tmpfile        = aufs_tmpfile
 };
------------------------------------------------------------------------------
BPM Camp - Free Virtual Workshop May 6th at 10am PDT/1PM EDT
Develop your own process in accordance with the BPMN 2 standard
Learn Process modeling best practices with Bonita BPM through live exercises
http://www.bonitasoft.com/be-part-of-it/events/bpm-camp-virtual- event?utm_
source=Sourceforge_BPM_Camp_5_6_15&utm_medium=email&utm_campaign=VA_SF

Reply via email to