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][email protected]> wrote:
Hi,
thank you for your response!
On Mon, Apr 6, 2015 at 6:30 AM, <[2][email protected]> 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:[email protected]
2. mailto:[email protected]
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
