Skip to contentSkip to conversation

VFS - the first pile

GIT PULL Al Viro 6 participants 14 messages Mar 15, 2011 – Mar 16, 2011 VFS

lore
Share & tools
lore

Al Viro

author maintainer
#0 lore raw

Let's try to start pushing early in the window this time...
There are several big ones in that:
* pathname resolution rewrite: trailing symlink resolution is
iterative in all cases, overall structure simplified a _lot_, do_lookup() turned into something readable, knowledge of RCU stuff localized, do_last() returned to handling non-create cases as well, NEED_REVAL_DOT got a lot saner.
* open-by-fhandle series by Aneesh
* O_PATH descriptors (basically, "location in namespace, not actually
opened from fs POV, can be dup'ed/passed around/etc. and used as starting points in ...at(2) kind of syscalls, but not used for any IO; that stuff allows to do sane pathname resolution in userland and avoids the need to add fsckloads of extra syscalls in open-by-fhandle series).

There will be more pull requests; this is just taking care of really heavy-weight stuff.

Please, pull from
git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs-2.6.git/ for-linus

Shortlog:
Al Viro (45):
compat breakage in preadv() and pwritev()
fix race in audit_get_nd()
kill path_lookup()
take RCU-dependent stuff around exec_permission() into a new helper sanitize path_walk() mess
merge path_init and path_init_rcu
merge component type recognition
untangle the "need_reval_dot" mess
pull dropping RCU on success of link_path_walk() into path_lookupat() pull security_inode_follow_link() into __do_follow_link() clean up the failure exits after __do_follow_link() in do_filp_open() Collect "operation mode" arguments of do_last() into a structure switch do_filp_open() to struct open_flags
separate -ESTALE/-ECHILD retries in do_filp_open() from real work kill out_dput: in link_path_walk()
pull handling of . and .. into inlined helper
clear RCU on all failure exits from link_path_walk()
make handle_dots() leave RCU mode on error
make nameidata_dentry_drop_rcu_maybe() always leave RCU mode get rid of the last LOOKUP_RCU dependencies in link_path_walk() get rid of nd->file
switch non-create side of open() to use of do_last()
kill __lookup_one_len()
sanitize pathname component hash calculation
expand finish_open() in its only caller
move may_open() from __open_name_create() to do_last()
do_last: unify may_open() call and everyting after it
fold __open_namei_create() and open_will_truncate() into do_last()
do_last: kill a rudiment of old ->d_revalidate() workaround
path_openat: clean ELOOP handling a bit
untangle do_lookup()
reduce vfs_path_lookup() to do_path_lookup()
open-style analog of vfs_path_lookup()
clean statfs-like syscalls up
New AT_... flag: AT_EMPTY_PATH
New kind of open files - "location only".
Allow O_PATH for symlinks
readlinkat(), fchownat() and fstatat() with empty relative pathnames Allow passing O_PATH descriptors via SCM_RIGHTS datagrams pull handling of one pathname component into a helper update nd->inode in __do_follow_link() instead of after do_follow_link() Make trailing symlink resolution in path_lookupat() iterative simplify link_path_walk() tail
Turn resolution of trailing symlinks iterative everywhere tidy the trailing symlinks traversal up

Aneesh Kumar K.V (12):
exportfs: Return the minimum required handle size
vfs: Add name to file handle conversion support
vfs: Add open by file handle support
fs: Don't allow to create hardlink for deleted file
fs: Remove i_nlink check from file system link callback
x86: Add new syscalls for x86_32
x86: Add new syscalls for x86_64
unistd.h: Add new syscalls numbers to asm-generic
vfs: Export file system uuid via /proc/<pid>/mountinfo
ext3: Copy fs UUID to superblock.
ext4: Copy fs UUID to superblock
fs: allow AT_EMPTY_PATH in linkat(), limit that to CAP_DAC_READ_SEARCH

Diffstat:

 arch/alpha/kernel/osf_sys.c                  |   36 +-
 arch/parisc/hpux/sys_hpux.c                  |   65 +-
 arch/powerpc/platforms/cell/spufs/syscalls.c |    2 +-
 arch/um/drivers/mconsole_kern.c              |   21 +-
 arch/x86/ia32/ia32entry.S                    |    2 +
 arch/x86/include/asm/unistd_32.h             |    4 +-
 arch/x86/include/asm/unistd_64.h             |    4 +
 arch/x86/kernel/syscall_table_32.S           |    2 +
 fs/Kconfig                                   |    2 +-
 fs/Makefile                                  |    2 +
 fs/btrfs/export.c                            |    8 +-
 fs/btrfs/inode.c                             |    3 -
 fs/compat.c                                  |   69 +-
 fs/exec.c                                    |   18 +-
 fs/exportfs/expfs.c                          |   11 +-
 fs/ext3/namei.c                              |    7 -
 fs/ext3/super.c                              |    1 +
 fs/ext4/namei.c                              |    7 -
 fs/ext4/super.c                              |    2 +
 fs/fat/inode.c                               |    4 +-
 fs/fcntl.c                                   |   37 +-
 fs/fhandle.c                                 |  265 +++++
 fs/file_table.c                              |   55 +-
 fs/fuse/inode.c                              |    4 +-
 fs/gfs2/export.c                             |    8 +-
 fs/internal.h                                |   13 +
 fs/isofs/export.c                            |    8 +-
 fs/jfs/namei.c                               |    3 -
 fs/namei.c                                   | 1498 +++++++++++---------------
 fs/namespace.c                               |   16 +
 fs/nfsctl.c                                  |   21 +-
 fs/ocfs2/export.c                            |    8 +-
 fs/ocfs2/refcounttree.c                      |    2 +-
 fs/open.c                                    |  126 ++-
 fs/reiserfs/inode.c                          |    7 +-
 fs/reiserfs/namei.c                          |    4 -
 fs/stat.c                                    |    7 +-
 fs/statfs.c                                  |  176 ++--
 fs/ubifs/dir.c                               |   18 -
 fs/udf/namei.c                               |    7 +-
 fs/xfs/linux-2.6/xfs_export.c                |    4 +-
 include/asm-generic/fcntl.h                  |    4 +
 include/asm-generic/unistd.h                 |    6 +-
 include/linux/exportfs.h                     |    9 +-
 include/linux/fcntl.h                        |    1 +
 include/linux/file.h                         |    2 +
 include/linux/fs.h                           |   19 +-
 include/linux/namei.h                        |    7 +-
 include/linux/syscalls.h                     |    8 +-
 init/Kconfig                                 |   12 +
 kernel/audit_watch.c                         |   85 +-
 kernel/sys_ni.c                              |    5 +
 kernel/sysctl_binary.c                       |   19 +-
 mm/shmem.c                                   |    4 +-
 net/core/scm.c                               |    2 +-
 net/unix/af_unix.c                           |    2 +-
 net/unix/garbage.c                           |    2 +-
 57 files changed, 1489 insertions(+), 1255 deletions(-)
#1 lore raw

Hi Al.

Quoted 5 lines

Shortlog:
Al Viro (45):
compat breakage in preadv() and pwritev()
fix race in audit_get_nd()
kill path_lookup()

....

It would be nice if you could start to prefix your changelog message with the sub-system they touch.

In an attempt to try to use the language you understand...

Why the hell do you continue making it so freaking difficult to see what sub-system your patchs touches.
All sane thinking persons has long since learned to prefix their changelogs with for example "vfs: bla bla" so sane humans has a chance to realize what they touches.
You could probarly do better than this :-)

Sam

author maintainer
#2 lore raw
In reply to Al Viro
Quoted 3 lines · Al Viro

Shortlog:
Al Viro (45):
compat breakage in preadv() and pwritev()

BTW, a question about rebases: is that kind of situation worth doing a rebase of the queue/backmerge/etc.?

This commit sits in the base of that branch, with branchpoint at Mar 10; the same thing had been applied in mainline at Mar 13. git merge does, of course, handle it just fine; I guess I should've posted a pull request back then instead of posting the patch - to be honest, I hadn't thought about that.

What's the prefered way of dealing with such situations? Leave as is? Rebase at the point where the patch went into mainline? Something else?

#3 lore raw
Quoted 3 lines · Al Viro

BTW, a question about rebases: is that kind of situation worth doing a rebase of the queue/backmerge/etc.?

No. If it's the same patch I already applied, it will generally merge cleanly, it's better to leave in the history than to rewrite things just to avoid some trivial duplication.

Linus

author maintainer
#4 lore raw
Quoted 8 lines · Linus Torvalds
Quoted 3 lines · Al Viro

BTW, a question about rebases: is that kind of situation worth doing a rebase of the queue/backmerge/etc.?

No. If it's the same patch I already applied, it will generally merge cleanly, it's better to leave in the history than to rewrite things just to avoid some trivial duplication.

It is and it does. OK, then...

#5 lore raw
In reply to Al Viro
Quoted 14 lines · Al Viro

Let's try to start pushing early in the window this time...
There are several big ones in that:
* pathname resolution rewrite: trailing symlink resolution is
iterative in all cases, overall structure simplified a _lot_, do_lookup() turned into something readable, knowledge of RCU stuff localized, do_last() returned to handling non-create cases as well, NEED_REVAL_DOT got a lot saner.
* open-by-fhandle series by Aneesh
* O_PATH descriptors (basically, "location in namespace, not actually
opened from fs POV, can be dup'ed/passed around/etc. and used as starting points in ...at(2) kind of syscalls, but not used for any IO; that stuff allows to do sane pathname resolution in userland and avoids the need to add fsckloads of extra syscalls in open-by-fhandle series).

Quoted 5 lines

There will be more pull requests; this is just taking care of really heavy-weight stuff.

Please, pull from
git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs-2.6.git/ for-linus

I think there's something not quite right in this tree. xfstests 005 (nested symlink test) fails on both ext4 and XFS with the following:

[   47.457457] EXT4-fs (vda): mounted filesystem with ordered data mode. Opts: acl,user_xattr
[   48.694685] ------------[ cut here ]------------
[   48.695168] kernel BUG at fs/dcache.c:436!
[   48.695568] invalid opcode: 0000 [#1] SMP 
[   48.695997] last sysfs file: /sys/devices/pci0000:00/0000:00:05.0/virtio2/block/vdb/removable
[   48.696835] CPU 0 
[   48.697021] Modules linked in:
[   48.697332] 
[   48.697332] Pid: 5851, comm: ln Not tainted 2.6.38-dgc+ #1096 Bochs Bochs
[   48.697332] RIP: 0010:[<ffffffff8118141a>]  [<ffffffff8118141a>] dput+0x28a/0x290
[   48.697332] RSP: 0018:ffff880079cf9ca8  EFLAGS: 00010246
[   48.697332] RAX: 0000000000000000 RBX: ffff8800710c38b0 RCX: 0000000000000000
[   48.697332] RDX: ffff88007f20ed20 RSI: ffff8800710c3928 RDI: 0000000000000246
[   48.697332] RBP: ffff880079cf9cd8 R08: 0000000000000000 R09: 0000000000000000
[   48.697332] R10: 0000000000000000 R11: 0000000000000000 R12: ffffffffffffffd8
[   48.697332] R13: ffff880079cde4c0 R14: ffff8800710c3910 R15: 0000000000000001
[   48.697332] FS:  00007fba3bcaf700(0000) GS:ffff88007f200000(0000) knlGS:0000000000000000
[   48.697332] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   48.697332] CR2: 00007fba3b8047d0 CR3: 0000000078415000 CR4: 00000000000006f0
[   48.697332] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[   48.697332] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
[   48.697332] Process ln (pid: 5851, threadinfo ffff880079cf8000, task ffff880079cde4c0)
[   48.697332] Stack:
[   48.697332]  ffff880079cf9dd8 ffff880079cf9d38 ffffffffffffffd8 ffff880079cde4c0
[   48.697332]  ffffffffffffffd8 0000000000000001 ffff880079cf9cf8 ffffffff8117352a
[   48.697332]  ffff880079cf9dd8 ffff880079cf9dd8 ffff880079cf9d98 ffffffff8117796b
[   48.697332] Call Trace:
[   48.697332]  [<ffffffff8117352a>] path_put+0x1a/0x30
[   48.697332]  [<ffffffff8117796b>] path_lookupat+0x2cb/0x770
[   48.697332]  [<ffffffff81177e43>] do_path_lookup+0x33/0x80
[   48.697332]  [<ffffffff81178ac9>] user_path_at+0x59/0xb0
[   48.697332]  [<ffffffff81b434a0>] ? do_page_fault+0x230/0x4e0
[   48.697332]  [<ffffffff8116d83c>] vfs_fstatat+0x4c/0x90
[   48.697332]  [<ffffffff8116d9ab>] vfs_stat+0x1b/0x20
[   48.697332]  [<ffffffff8116d9d4>] sys_newstat+0x24/0x50
[   48.697332]  [<ffffffff81b3eb09>] ? trace_hardirqs_on_thunk+0x3a/0x3f
[   48.697332]  [<ffffffff8103cf42>] system_call_fastpath+0x16/0x1b
[   48.697332] Code: 48 89 93 d0 00 00 00 4c 89 a0 d8 01 00 00 48 8b 83 b0 00 00 00 ff 80 e8 01 00 00 ff 05 54 4a df 00 e8 6b e4 9b 00 e9 77 fe ff ff <0f> 0b eb fe 66  
[   48.697332] RIP  [<ffffffff8118141a>] dput+0x28a/0x290
[   48.697332]  RSP <ffff880079cf9ca8>
[   48.719193] ---[ end trace bdba6e6dd185beb8 ]---

Cheers,

Dave.

Signature · 2 lines
Dave Chinner
[email protected]
author maintainer
#6 lore raw
Quoted 2 lines · Dave Chinner

I think there's something not quite right in this tree. xfstests 005 (nested symlink test) fails on both ext4 and XFS with the following:

Yeah... Brown paperbag time - it's actually a missing piece that got glued with a commit a bit later in queue ;-/ I've dropped it into for-linus for the morning push; in the meanwhile, the patch is below.

fs/namei.c +3 -4
index 0a601ca..b912b7a 100644
@@ -753,9 +753,11 @@ follow_link(struct path *link, struct nameidata *nd, void **p)
 
 	BUG_ON(nd->flags & LOOKUP_RCU);
 
+	if (link->mnt == nd->path.mnt)
+		mntget(link->mnt);
+
 	if (unlikely(current->total_link_count >= 40)) {
 		*p = ERR_PTR(-ELOOP); /* no ->put_link(), please */
-		path_put_conditional(link, nd);
 		path_put(&nd->path);
 		return -ELOOP;
 	}
@@ -765,9 +767,6 @@ follow_link(struct path *link, struct nameidata *nd, void **p)
 	touch_atime(link->mnt, dentry);
 	nd_set_link(nd, NULL);
 
-	if (link->mnt == nd->path.mnt)
-		mntget(link->mnt);
-
 	error = security_inode_follow_link(link->dentry, nd);
 	if (error) {
 		*p = ERR_PTR(error); /* no ->put_link(), please */
author maintainer
#7 lore raw
Quoted 8 lines · Al Viro
Quoted 2 lines · Dave Chinner

I think there's something not quite right in this tree. xfstests 005 (nested symlink test) fails on both ext4 and XFS with the following:

Yeah... Brown paperbag time - it's actually a missing piece that got glued with a commit a bit later in queue ;-/ I've dropped it into for-linus for the morning push; in the meanwhile, the patch is below.

BTW, you want to update 005 in there - we are back to correct "maximum is 40 symlinks total, 8 levels on nesting" for all syscalls. Add the 41st symlink to your chain in testcase ;-)

#8 lore raw
Quoted 3 lines · Al Viro

BTW, you want to update 005 in there - we are back to correct "maximum is 40 symlinks total, 8 levels on nesting" for all syscalls. Add the 41st symlink to your chain in testcase ;-)

Unless there's a way to read out these limits, I'm not sure it's a good idea to add a test like that to xfstests --- it's too fragile since at some point we might change what those limits might be.

Also, xfstests is primarily intended to be a file system level stress tester testing for correctness, and issues of whether we blow up on the 40th, 41st, or 42nd symlink seems more like an ABI issue --- and even there I'm not sure the ABI specification should be quite that detailed over what's allowed and not allowed.

-- Ted

#9 lore raw
Quoted 11 lines · Theodore Tso
Quoted 3 lines · Al Viro

BTW, you want to update 005 in there - we are back to correct "maximum is 40 symlinks total, 8 levels on nesting" for all syscalls. Add the 41st symlink to your chain in testcase ;-)

Unless there's a way to read out these limits, I'm not sure it's a good idea to add a test like that to xfstests --- it's too fragile since at some point we might change what those limits might be.

Quoted 5 lines

Also, xfstests is primarily intended to be a file system level stress tester testing for correctness, and issues of whether we blow up on the 40th, 41st, or 42nd symlink seems more like an ABI issue --- and even there I'm not sure the ABI specification should be quite that detailed over what's allowed and not allowed.

That's not what it tests anyway. It tests that we get ELOOP at some point, and do not blow the stack. Which is someting that older Linux code used to do.

author maintainer
#10 lore raw
Quoted 18 lines · Christoph Hellwig
Quoted 11 lines · Theodore Tso
Quoted 3 lines · Al Viro

BTW, you want to update 005 in there - we are back to correct "maximum is 40 symlinks total, 8 levels on nesting" for all syscalls. Add the 41st symlink to your chain in testcase ;-)

Unless there's a way to read out these limits, I'm not sure it's a good idea to add a test like that to xfstests --- it's too fragile since at some point we might change what those limits might be.

Quoted 5 lines

Also, xfstests is primarily intended to be a file system level stress tester testing for correctness, and issues of whether we blow up on the 40th, 41st, or 42nd symlink seems more like an ABI issue --- and even there I'm not sure the ABI specification should be quite that detailed over what's allowed and not allowed.

Quoted 3 lines

That's not what it tests anyway. It tests that we get ELOOP at some point, and do not blow the stack. Which is someting that older Linux code used to do.

Yes. See patch upthread (or in for-linus). There are two parts in that test; *both* would actually trigger the b0rkage in the last commit of what got merged - the only reason why the first one (long chain) did not was that the limit got fixed and -ELOOP was no longer triggered. Symlink to itself did, of course, trigger it - complete with oops.

#11 lore raw
Quoted 5 lines · Al Viro

Yes. See patch upthread (or in for-linus). There are two parts in that test; *both* would actually trigger the b0rkage in the last commit of what got merged - the only reason why the first one (long chain) did not was that the limit got fixed and -ELOOP was no longer triggered. Symlink to itself did, of course, trigger it - complete with oops.

Yes. I have a patch to increase the limit tested in 005 and it now passes fine.

#12 lore raw
In reply to Al Viro

This blows up on xfstests 005 for me:

005 0s ...

[   25.075985] ------------[ cut here ]------------
[   25.076043] kernel BUG at /home/hch/work/linux-2.6/fs/dcache.c:440!
[   25.076043] invalid opcode: 0000 [#1] SMP 
[   25.076043] last sysfs file: /sys/devices/pci0000:00/0000:00:05.0/virtio1/block/vdb/removable
[   25.076043] Modules linked in:
[   25.076043] 
[   25.076043] Pid: 6263, comm: touch Not tainted 2.6.38+ #77 Bochs Bochs
[   25.076043] EIP: 0060:[<c021d36e>] EFLAGS: 00010246 CPU: 0
[   25.076043] EIP is at dput+0x22e/0x240
[   25.076043] EAX: 00000000 EBX: f54b9f14 ECX: 00000000 EDX: 00005959
[   25.076043] ESI: ffffffd8 EDI: ffffffd8 EBP: f5361e84 ESP: f5361e68
[   25.076043]  DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
[   25.076043] Process touch (pid: 6263, ti=f5360000 task=f59c23f0 task.ti=f5360000)
[   25.076043] Stack:
[   25.076043]  0000000c f54b9f14 f5361eec f54b9f60 f5361eb8 ffffffd8 ffffffd8 f5361e90
[   25.076043]  c02129ce f5361eec f5361ed8 c0215702 f644d000 f5361ec8 f5361f78 f644d000
[   25.076043]  f59c23f0 f5361f00 f59c23f0 c04c0aa0 f5a18180 f54b9f14 f5a18180 f54b9f14
[   25.076043] Call Trace:
[   25.076043]  [<c02129ce>] path_put+0xe/0x20
[   25.076043]  [<c0215702>] path_openat+0x1d2/0x310
[   25.076043]  [<c04c0aa0>] ? xfs_vn_put_link+0x0/0x20
[   25.076043]  [<c0215920>] do_filp_open+0x30/0x80
[   25.076043]  [<c02205cc>] ? alloc_fd+0x6c/0xf0
[   25.076043]  [<c0213171>] ? getname_flags+0x61/0xe0
[   25.076043]  [<c020915b>] do_sys_open+0xeb/0x1c0
[   25.076043]  [<c0209299>] sys_open+0x29/0x40
[   25.076043]  [<c08f62b9>] syscall_call+0x7/0xb
[   25.076043] Code: 89 72 04 89 53 60 8d 50 7c 89 53 64 89 70 7c 8b 43 54 83 80 84 00 00 00 01 83 05 a8 32 c4 c0 01 fe 05 00 4c c3 c0 e9 8b fe ff ff <0f> 0b eb fe 8d b4 26 00 00 00 00 8d bc 27 00 00 00 00 55 89 e5 
#13 lore raw
Quoted 1 line · Christoph Hellwig

This blows up on xfstests 005 for me:

Discard it - I saw Dave already posted it.