Bug 719607
Summary: | automount does not work on "ls -ld /misc/cd" but it does work on "test -d /misc/cd" | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Joachim Jaeckel <bugzilla-2004> | ||||||||
Component: | autofs | Assignee: | Ian Kent <ikent> | ||||||||
Status: | CLOSED WONTFIX | QA Contact: | Fedora Extras Quality Assurance <extras-qa> | ||||||||
Severity: | medium | Docs Contact: | |||||||||
Priority: | unspecified | ||||||||||
Version: | 15 | CC: | bugzilla-2004, dhowells, ikent, jlayton, leonardo, mjt | ||||||||
Target Milestone: | --- | ||||||||||
Target Release: | --- | ||||||||||
Hardware: | x86_64 | ||||||||||
OS: | Linux | ||||||||||
Whiteboard: | |||||||||||
Fixed In Version: | Doc Type: | Bug Fix | |||||||||
Doc Text: | Story Points: | --- | |||||||||
Clone Of: | Environment: | ||||||||||
Last Closed: | 2012-08-07 20:20:23 UTC | Type: | --- | ||||||||
Regression: | --- | Mount Type: | --- | ||||||||
Documentation: | --- | CRM: | |||||||||
Verified Versions: | Category: | --- | |||||||||
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||||||
Cloudforms Team: | --- | Target Upstream Version: | |||||||||
Embargoed: | |||||||||||
Bug Depends On: | 692823, 692982, 720318, 720325, 733978, 733979 | ||||||||||
Bug Blocks: | |||||||||||
Attachments: |
|
(In reply to comment #0) > Created attachment 511691 [details] > Testprogram with lstat and stat > > Description of problem: > > automount does not mount when issuing e.g. > ls -l /misc/cd > but it works when issuing > test -d /misc/cd > > With strace i see the difference between ls and test: > ls calls lstat(2), > test calls stat(2) > > I wrote a small C program to reproduce this issue, see attachment. > lstat("/misc/cd",...) fails > stat("/misc/cd",...) trigges automount, /misc/cd will be mounted OK, that's the second time this behaviour has been reported against 2.6.38+ kernels. The semantics have changed a little in 2.6.38+ but, according to the code the opposite should happen which is puzzling me. I must be missing some other case where the behaviour has changed. Let me have a think about it and a look around. > > Version-Release number of selected component (if applicable): > autofs-5.0.5-38.fc15.x86_64 Your using the default installed autofs configuration, right? > kernel-2.6.38.8-32.fc15.x86_64 > > > How reproducible: > always > > Steps to Reproduce: > 1. activate default /etc/auto.misc in /etc/auto.master > 2. service autofs restart > 3. ls -ld /misc/cd Adding a "/" on the end should cause the mount to occur. > 4. test -d /misc/cd > > or: > 3. Compile the test program: make automount_test > 4. run the testprogram: ./automount_test And the same here. Ian > Your using the default installed autofs configuration, right? Yes. > Adding a "/" on the end should cause the mount to occur. No: ls -l /misc/cd/ # no mount occurs But: ls -l /misc/cd/xyz # mount occurs, xyz may be any characters Simple way to see the lstat: [root@vega ~]# service autofs restart Restarting autofs (via systemctl): [ OK ] [root@vega ~]# strace ls -l /misc/cd 2>&1 | grep /misc/ execve("/bin/ls", ["ls", "-l", "/misc/cd"], [/* 28 vars */]) = 0 lstat("/misc/cd", 0x256d1b0) = -1 ENOENT (No such file or directory) write(2, "cannot access /misc/cd", 22cannot access /misc/cd) = 22 [root@vega ~]# strace ls -l /misc/cd/ 2>&1 | grep /misc/ execve("/bin/ls", ["ls", "-l", "/misc/cd/"], [/* 28 vars */]) = 0 lstat("/misc/cd/", 0xe481b0) = -1 ENOENT (No such file or directory) write(2, "cannot access /misc/cd/", 23cannot access /misc/cd/) = 23 [root@vega ~]# strace ls -l /misc/cd/abc 2>&1 | grep /misc/ execve("/bin/ls", ["ls", "-l", "/misc/cd/abc"], [/* 28 vars */]) = 0 lstat("/misc/cd/abc", 0x26001b0) = -1 ENOENT (No such file or directory) write(2, "cannot access /misc/cd/abc", 26cannot access /misc/cd/abc) = 26 # last statement will mount, lstat fails because /abc doesnt exist on my CD Joachim Can you try the following: strace -etrace=lstat,stat,getxattr,lgetxattr ls -l /misc/cd and paste the result? Also, can you paste the result of the following: rpm -q libacl coreutils What used to happen was that ls called into libacl to check the ACL - and that used getxattr() thereby causing the mount anyway. This was found recently [bug 692823] and the fix may have gotten into Fedora 15, though not yet Fedora 14, in which case this will no longer cause an automount. The idea is to prevent "ls -l /misc" causing an automount storm in /misc has a lot of mountpoints in it. The problem is that "ls -l /misc" and "ls -l /misc/cd" both lstat, lgetxattr (and may getxattr if an old libacl) /misc/cd - so you can't tell them apart. (In reply to comment #3) > strace -etrace=lstat,stat,getxattr,lgetxattr ls -l /misc/cd [root@vega ~]# strace -etrace=lstat,stat,getxattr,lgetxattr ls -l /misc/cd lstat("/misc/cd", 0x11341b0) = -1 ENOENT (No such file or directory) ls: cannot access /misc/cd: No such file or directory [root@vega ~]# > rpm -q libacl coreutils libacl-2.2.49-9.fc15.x86_64 coreutils-8.10-2.fc15.x86_64 > What used to happen was that ls called into libacl to check the ACL - and that > used getxattr() thereby causing the mount anyway. This was found recently [bug > 692823] and the fix may have gotten into Fedora 15, though not yet Fedora 14, > in which case this will no longer cause an automount. The idea is to prevent > "ls -l /misc" causing an automount storm in /misc has a lot of mountpoints in > it. > > The problem is that "ls -l /misc" and "ls -l /misc/cd" both lstat, lgetxattr > (and may getxattr if an old libacl) /misc/cd - so you can't tell them apart. I have several mounts defined in /etc/auto.misc But my /misc directory contains only subdirectories for the mount points wich are actually mounted by autofs. When automount has dismounted all fs after timeout, /misc is empty. Joachim (In reply to comment #4) > I have several mounts defined in /etc/auto.misc > But my /misc directory contains only subdirectories for the mount points wich > are actually mounted by autofs. > When automount has dismounted all fs after timeout, /misc is empty. Yes, in this case, with previous kernels, automount would always mount when called with "/misc/<key>". If the configuration had BROWSE_MODE="yes" or if that configuration entry was commented out then "/misc/<key>" would not cause a mount regardless of whether you used the l variant of the system call or not. (In reply to comment #2) > > Your using the default installed autofs configuration, > right? > Yes. > > > Adding a "/" on the end should cause the mount to occur. > No: > ls -l /misc/cd/ # no mount occurs > But: > ls -l /misc/cd/xyz # mount occurs, xyz may be any characters > > Simple way to see the lstat: > [root@vega ~]# service autofs restart > Restarting autofs (via systemctl): [ OK ] > [root@vega ~]# strace ls -l /misc/cd 2>&1 | grep /misc/ > execve("/bin/ls", ["ls", "-l", "/misc/cd"], [/* 28 vars */]) = 0 > lstat("/misc/cd", 0x256d1b0) = -1 ENOENT (No such file or directory) > write(2, "cannot access /misc/cd", 22cannot access /misc/cd) = 22 > [root@vega ~]# strace ls -l /misc/cd/ 2>&1 | grep /misc/ > execve("/bin/ls", ["ls", "-l", "/misc/cd/"], [/* 28 vars */]) = 0 > lstat("/misc/cd/", 0xe481b0) = -1 ENOENT (No such file or directory) > write(2, "cannot access /misc/cd/", 23cannot access /misc/cd/) = 23 > [root@vega ~]# strace ls -l /misc/cd/abc 2>&1 | grep /misc/ > execve("/bin/ls", ["ls", "-l", "/misc/cd/abc"], [/* 28 vars */]) = 0 > lstat("/misc/cd/abc", 0x26001b0) = -1 ENOENT (No such file or directory) > write(2, "cannot access /misc/cd/abc", 26cannot access /misc/cd/abc) = 26 > > # last statement will mount, lstat fails because /abc doesnt exist on my CD OK, I'll need to think about this a bit (and run some tests) because not all of these cases are what I expect to happen with the current kernel code. Ian David I seem to be misunderstanding the test in fs/namei.c:follow_automount() so I've been looking at it. Forgetting what the test actually does for a bit and thinking about what we should do I've come up with this. First the terminology, the "follow link" in system call descriptions equates to "follow mount" for (us in) vfs-automount. So for system calls that do not follow the link (the l calls) we should work on the mountpoint. So for this case we shouldn't call ->d_automount(). In a semantically perfect world system calls that do follow the link should call ->d_automount(). But we know that isn't practical due to mount storms when long listing the contents of directories that contain existing mount point directories because user space will do a bunch of stat, getxattr and the like, and, as we have seen maybe lgetxattr. Assuming the above is what we want, returning to the test, I think the test in follow_automount() is not quite right and the above amounts to saying the && should actually be an || like: if (!(flags & LOOKUP_FOLLOW) || !(flags & (LOOKUP_CONTINUE | LOOKUP_DIRECTORY | LOOKUP_OPEN | LOOKUP_CREATE))) return -EISDIR; which says, if we are not following a link return -EISDIR, if we are following a link return -EISDIR if we are not evaluating an intermediate path component or we are have not been called by an open or create type system call, otherwise call ->d_automount(). I think this matches the previous kernel semantics for positive dentrys. Have I still got this wrong somehow? If I haven't we can close 692823 as NOTABUG with this change. The other case, for negative dentrys, which is the cause of the problem here, the situation is a bit different. For NFS, and I believe CIFS (Jeff?), the xdev dentrys will always be positive when we get to follow them since they must have a directory within the mount to cross. What's the situation for AFS David? But, in previous kernels, autofs would unconditionally trigger a mount in ->lookup() which amounts to saying vfs-automount() should always call ->d_automount() for negative dentrys (at least for autofs). IOW the test should be: if (path->dentry) { if (!(flags & LOOKUP_FOLLOW) || !(flags & (LOOKUP_CONTINUE | LOOKUP_DIRECTORY | LOOKUP_OPEN | LOOKUP_CREATE))) return -EISDIR; } } or equivalent. What do you guys think about my reasoning and would this break AFS, CIFS or NFS? Ian (In reply to comment #7) Oops! > The other case, for negative dentrys, which is the cause of the > problem here, the situation is a bit different. For NFS, and I > believe CIFS (Jeff?), the xdev dentrys will always be positive > when we get to follow them since they must have a directory within > the mount to cross. What's the situation for AFS David? > > But, in previous kernels, autofs would unconditionally trigger a > mount in ->lookup() which amounts to saying vfs-automount() > should always call ->d_automount() for negative dentrys (at least > for autofs). IOW the test should be: > > if (path->dentry) { That's path->dentry->d_inode, of course. > if (!(flags & LOOKUP_FOLLOW) || > !(flags & (LOOKUP_CONTINUE | LOOKUP_DIRECTORY | > LOOKUP_OPEN | LOOKUP_CREATE))) > return -EISDIR; > } And this } isn't needed. > } (In reply to comment #7) > David I seem to be misunderstanding the test in > fs/namei.c:follow_automount() so I've been looking at it. To make it easier to understand, invert the logic and use gotos: if (flags & LOOKUP_FOLLOW) goto do_mount; if (flags & (LOOKUP_CONTINUE | LOOKUP_DIRECTORY | LOOKUP_OPEN | LOOKUP_CREATE)) goto do_mount; return -EISDIR; do_mount: You have to remember that LOOKUP_FOLLOW is !AT_SYMLINK_NOFOLLOW, so you can read the original as: if AT_SYMLINK_NOFOLLOW _is_ set _and_ we are at the end of the path (!CONTINUE), we're not trying to open a directory (!DIRECTORY) and we're not trying to open/create a file (!OPEN/!CREATE), _then_ use the mountpoint dir - otherwise use the mounted directory. This results in the following: > ... because user space will do a bunch of stat, getxattr > and the like, and, as we have seen maybe lgetxattr. Just to note, stat/getxattr -> LOOKUP_FOLLOW; lstat/lgetxattr -> don't set it. > Assuming the above is what we want, returning to the test, I > think the test in follow_automount() is not quite right and > the above amounts to saying the && should actually be an || > like: > > if (!(flags & LOOKUP_FOLLOW) || > !(flags & (LOOKUP_CONTINUE | LOOKUP_DIRECTORY | > LOOKUP_OPEN | LOOKUP_CREATE))) > return -EISDIR; > > which says, if we are not following a link return -EISDIR, if > we are following a link return -EISDIR if we are not evaluating > an intermediate path component or we are have not been called > by an open or create type system call, otherwise call > ->d_automount(). Unfortunately, it doesn't work like that. What your change does is prevent _all_ automounts (even on intermediate components) if LOOKUP_FOLLOW is not set. > But, in previous kernels, autofs would unconditionally trigger a > mount in ->lookup() which amounts to saying vfs-automount() > should always call ->d_automount() for negative dentrys (at least > for autofs). IOW the test should be: > > if (path->dentry) { > if (!(flags & LOOKUP_FOLLOW) || > !(flags & (LOOKUP_CONTINUE | LOOKUP_DIRECTORY | > LOOKUP_OPEN | LOOKUP_CREATE))) > return -EISDIR; > } > } > > or equivalent. I think what you probably want is to further qualify the original statement: if (!(flags & LOOKUP_FOLLOW) && path->dentry->d_inode && !(flags & (LOOKUP_CONTINUE | LOOKUP_DIRECTORY | LOOKUP_OPEN | LOOKUP_CREATE))) return -EISDIR; Created attachment 511876 [details]
Unconditionally automount on negative dentries
(In reply to comment #9) > > > > which says, if we are not following a link return -EISDIR, if > > we are following a link return -EISDIR if we are not evaluating > > an intermediate path component or we are have not been called > > by an open or create type system call, otherwise call > > ->d_automount(). > > Unfortunately, it doesn't work like that. What your change does is prevent > _all_ automounts (even on intermediate components) if LOOKUP_FOLLOW is not set. > Yes, you're right. > > But, in previous kernels, autofs would unconditionally trigger a > > mount in ->lookup() which amounts to saying vfs-automount() > > should always call ->d_automount() for negative dentrys (at least > > for autofs). IOW the test should be: > > > > if (path->dentry) { > > if (!(flags & LOOKUP_FOLLOW) || > > !(flags & (LOOKUP_CONTINUE | LOOKUP_DIRECTORY | > > LOOKUP_OPEN | LOOKUP_CREATE))) > > return -EISDIR; > > } > > } > > > > or equivalent. > > I think what you probably want is to further qualify the original statement: > > if (!(flags & LOOKUP_FOLLOW) && path->dentry->d_inode && > !(flags & (LOOKUP_CONTINUE | LOOKUP_DIRECTORY | > LOOKUP_OPEN | LOOKUP_CREATE))) > return -EISDIR; Yes, that would resolve the problem here. But, from the testing I did, there are cases where the logic is inverted, aka. stat() causes a mount and lstat() does not, which I thought is the opposite to what we were after. Let me build a kernel with the patch above and test again, it may just be the negative dentry that is causing the confusion. Ian (In reply to comment #11) > > > > I think what you probably want is to further qualify the original statement: > > > > if (!(flags & LOOKUP_FOLLOW) && path->dentry->d_inode && > > !(flags & (LOOKUP_CONTINUE | LOOKUP_DIRECTORY | > > LOOKUP_OPEN | LOOKUP_CREATE))) > > return -EISDIR; > > Yes, that would resolve the problem here. > > But, from the testing I did, there are cases where the logic is > inverted, aka. stat() causes a mount and lstat() does not, which > I thought is the opposite to what we were after. > > Let me build a kernel with the patch above and test again, it > may just be the negative dentry that is causing the confusion. My thinking was backward, which I realized after looking at bug 692823 again. The recommend patch does seem to do the trick, as long as I use updated coreutils (bug 692823) and acl (bug 692982) packages. The test kernel I used should still be available here: https://koji.fedoraproject.org/koji/taskinfo?taskID=3186328 Try it out and we'll work out from there if you need updated coreutils and acl packages. You probably don't need them unless you are using the browse autofs mount option and you have maps that are largish is size. Ian > The test kernel I used should still be available here:
> https://koji.fedoraproject.org/koji/taskinfo?taskID=3186328
>
> Try it out and we'll work out from there if you need updated
> coreutils and acl packages. You probably don't need them unless
> you are using the browse autofs mount option and you have maps
> that are largish is size.
>
> Ian
Cannot test it this week because i am on holidays. Will be back Jul 19.
Joachim
Created attachment 512209 [details]
Unconditionally automount on negative dentries V2
Simply switch the automount negative dentry check to be the second if-statement as the the LOOKUP_* flag check is much more likely to jump.
> My thinking was backward, which I realized after looking at > bug 692823 again. > > The recommend patch does seem to do the trick, as long as I > use updated coreutils (bug 692823) and acl (bug 692982) packages. > > The test kernel I used should still be available here: > https://koji.fedoraproject.org/koji/taskinfo?taskID=3186328 > > Try it out and we'll work out from there if you need updated > coreutils and acl packages. You probably don't need them unless > you are using the browse autofs mount option and you have maps > that are largish is size. > > Ian For testing i installed the test kernel from the above url. kernel-2.6.38.8-32.bz708039.2.fc15.x86_64 libacl-2.2.49-9.fc15.x86_64 coreutils-8.10-2.fc15.x86_64 Behaviour is as expected: automount is working again with ls -l /misc/cd Joachim I come across this issue too, after trying to bisect and realizing the first released "bad" kernel is 2.6.38. I tested this patch on 2.6.38 and 3.0 and found that it works as expected, restoring autofs functionality as it were before. Fix will be in 2.6.40.3-2 Tested with actual Fedora 15, kernel-2.6.40.4-5.fc15.x86_64 Resolved. This message is a notice that Fedora 15 is now at end of life. Fedora has stopped maintaining and issuing updates for Fedora 15. It is Fedora's policy to close all bug reports from releases that are no longer maintained. At this time, all open bugs with a Fedora 'version' of '15' have been closed as WONTFIX. (Please note: Our normal process is to give advanced warning of this occurring, but we forgot to do that. A thousand apologies.) Package Maintainer: If you wish for this bug to remain open because you plan to fix it in a currently maintained version, feel free to reopen this bug and simply change the 'version' to a later Fedora version. Bug Reporter: Thank you for reporting this issue and we are sorry that we were unable to fix it before Fedora 15 reached end of life. If you would still like to see this bug fixed and are able to reproduce it against a later version of Fedora, you are encouraged to click on "Clone This Bug" (top right of this page) and open it against that version of Fedora. Although we aim to fix as many bugs as possible during every release's lifetime, sometimes those efforts are overtaken by events. Often a more recent Fedora release includes newer upstream software that fixes bugs or makes them obsolete. The process we are following is described here: http://fedoraproject.org/wiki/BugZappers/HouseKeeping |
Created attachment 511691 [details] Testprogram with lstat and stat Description of problem: automount does not mount when issuing e.g. ls -l /misc/cd but it works when issuing test -d /misc/cd With strace i see the difference between ls and test: ls calls lstat(2), test calls stat(2) I wrote a small C program to reproduce this issue, see attachment. lstat("/misc/cd",...) fails stat("/misc/cd",...) trigges automount, /misc/cd will be mounted Version-Release number of selected component (if applicable): autofs-5.0.5-38.fc15.x86_64 kernel-2.6.38.8-32.fc15.x86_64 How reproducible: always Steps to Reproduce: 1. activate default /etc/auto.misc in /etc/auto.master 2. service autofs restart 3. ls -ld /misc/cd 4. test -d /misc/cd or: 3. Compile the test program: make automount_test 4. run the testprogram: ./automount_test Actual results: (restart autofs) [jj@vega ~]]$ ls -ld /misc/cd ls: cannot access /misc/cd: No such file or directory [jj@vega ~]]$ test -d /misc/cd [jj@vega ~]]$ ls -ld /misc/cd dr-xr-xr-x 13 root root 4096 Apr 27 12:49 /misc/cd [jj@vega ~]]$ (restart autofs) [jj@vega automount]$ make automount_test cc automount_test.c -o automount_test [jj@vega automount]$ ./automount_test lstat rc=-1 ls: cannot access /misc/cd: No such file or directory stat rc=0 dr-xr-xr-x 13 root root 4096 Apr 27 12:49 /misc/cd lstat rc=0 dr-xr-xr-x 13 root root 4096 Apr 27 12:49 /misc/cd [jj@vega automount]$ Expected results: ls should trigger automount (worked in former versions) Additional info: