Repository navigation
os.path.ismounts: further situations it doesn't detect correctly #96328
Copy link
Copy link
Open
Labels
docsDocumentation in the Doc dirDocumentation in the Doc dirtype-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error
Description
Bug report
I had a look at os.path.ismount():
Lines 186 to 216 in 43a6dea
| def ismount(path): | |
| """Test whether a path is a mount point""" | |
| try: | |
| s1 = os.lstat(path) | |
| except (OSError, ValueError): | |
| # It doesn't exist -- so not a mount point. :-) | |
| return False | |
| else: | |
| # A symlink can never be a mount point | |
| if stat.S_ISLNK(s1.st_mode): | |
| return False | |
| if isinstance(path, bytes): | |
| parent = join(path, b'..') | |
| else: | |
| parent = join(path, '..') | |
| parent = realpath(parent) | |
| try: | |
| s2 = os.lstat(parent) | |
| except (OSError, ValueError): | |
| return False | |
| dev1 = s1.st_dev | |
| dev2 = s2.st_dev | |
| if dev1 != dev2: | |
| return True # path/.. on a different device as path | |
| ino1 = s1.st_ino | |
| ino2 = s2.st_ino | |
| if ino1 == ino2: | |
| return True # path/.. is the same i-node as path | |
| return False |
And it may have some issues:
- The documentation already says, that it isn't capable of detecting bind-mounts of some filesystem on the very same filesystem. While it's good that it mentions this, the results of
ismount()is IMO still simply wrong in these cases, cause it is a mountpoint. - The same problem in the detection algorithm is not only limited to bind-mounts, but also to any other mount points, where the path’s
st_devequals that of its parent. For example, btrfs subvolumes. This is however not documented.
Looking a the code, it effectively does: lstat(), check whether it’s a symlink, lstat() the parent, check whether the device numbers are different (then it's a mountpoint), check whether the inodes are the same.
Now I thought about the latter, and what it is meant for.
The only case I could think of at first is /, where /.. is the same inode. [Is there any other?]
- This however already requires, that not only the inodes are the same, but also the device numbers. which is given because of the order of these checks, however, I'd suggest to comment that accordingly.
- If the above were the only case, wouldn't it be faster to simply check the pathname for being
"/"orb"/". Would perhaps require somerealpath()though, to also catch things like"/../.."and so on. - However, I found that
/is not the only case: Within a chroot, the chroot's/may (or may not) be a mountpoint - the problem is just, the current algorithm doesn't detect this either. Instead, it wrongly claims that a chroot’s "/" would be a mountpoint, even when it isn't.
Not sure what one could do about all that… /etc/mtab is dead... /proc/mounts may not be available on all POSIX systems and even on Linux it's nowadays rather superseded by /proc/self/mountinfo.
Your environment
- CPython versions tested on: 3.10.6
- Operating system and architecture: Debian sid
Cheers,
Chris.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
docsDocumentation in the Doc dirDocumentation in the Doc dirtype-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error
Projects
- StatusShow more project fieldsTodo
Activity
calestyo commentedon Aug 28, 2022
A further bug in
ismount()seems to be e.g.:assuming that
/root/isn't accessible by the calling user.The function reports
False, however, it may very well be a mountpoint, just the user cannot see.eryksun commentedon Aug 28, 2022
If direct access is denied, then
exists(),lexists(),islink(),isdir(), andisfile()also return false. They depend on eitherstat()orlstat().The point about btfrs subvolumes is a duplicate of #81520. Returning true for btrfs subvolumes might be useful. The fact that
rename()to a another subvolume fails withEXDEV(i.e. invalid cross-device link) makes it a peculiar case if it's not a mount point. Either way, it would help to call attention to this in the docs.According to POSIX, a mount point is "[e]ither the system root directory or a directory for which the
st_devfield of structurestatdiffers from that of its parent directory". This is all thatposixpath.ismount()implements. In Linux, a bind mount point may not satisfy the requirement for the parent to be on a different device (unless bindfs is used). That said, I tested arename()across a bind mount point, and it does fail withEXDEVeven when the source and destination are on the same device.Regarding
chroot(), returning true for the root directory in the current process possibly fits the POSIX definition of a mount point, depending on the interpretation of "system" in "system root directory". It could be the system as seen by a process. OTOH, POSIX doesn't even definechroot(), so a literal interpretation of "system root directory" in the spec would be the real root directory. How would one detect thechroot()case in a cross-platform way? One can't rely on a particular inode number for the root directory in a filesystem. When does this case matter?calestyo commentedon Aug 28, 2022
Aren't these results then also bogus?! If one does e.g.
exists()(where the behaviour is however documented, at least, however for the others, e.g.isdir()it's not really) one wants to know whether it exists or not - not that it doesn’t when one actually just hadn't the permissions to check.Same für btrfs and #81520 ... having it documented is better than nothing, but it would still not match the semantics of what one could reasonably expect from functions named
isdir(),exists()orismount()I'm not so sure whether strictly following the POSIX definition is appropriate here. That is probably decades old and back then there weren't concepts like subvolumes or bind mounts, or at least POSIX didn't really care too much about them.
It does at least not match what an OS may nowadays consider a mountpoint.
Imagine you write a tool, that e.g. deletes a file tree and wants to check before whether anything is a mounpoiunt (cause if so, it couldn't delete that)… this would already fail with the current
ismount().Or imagine one wants to implement a true
-xdevoption and needs to check whether a dir is a mounpoint (though admittedly,find, as of now, also fails on this).I don't think there's a real portable way (which is because POSIX doesn't really standardise these things too deeply)... the inode number doesn't work, not even under Linux (again btrfs, e.g.).
Use case… well I don't know… perhaps something like: if some dir is a mountpoint, one want's to set some fs properties or maybe remount,ro it… so it should work when / is truly the mountpoint but not e.g. in a chroot, unless the chroot is in its own filesystem.
Maybe one could add non-portable ways of determining (e.g. using
/proc/self/mountinfo)… problem is though, whether this can be made fast… and fall back to the current algo if there is none for the current platform.One could then also add a
strictparameter that lets the function fail in cases where it cannot be said for sure.calestyo commentedon Aug 28, 2022
Some more things, which are however not possible bugs:
First:
cpython/Lib/posixpath.py
Lines 194 to 196 in e860e52
Edit: this part is bogus see the 2nd comment after this one
AFAIU, this is in principle not necessary, and even without it, the code should never consider a symlink to be a mountpoint. `lstat()` is used everywhere, so the symlink’s device ID should always be different from it's parent directory's.If that piece of code is however just for speed up (i.e. to save the
realpath()and thestat()on the parent directory - which would be a good thing), then why not simply checking forS_ISDIR? No file type other than directory should ever be able to be a directory.(I could provide a patch if desired).
calestyo commentedon Aug 28, 2022
Second:
cpython/Lib/posixpath.py
Line 202 in e860e52
I already tried to mail the two authors of the commit 750018b (issue #46718) which introduced that.
The call to
realpath()makes the execution ofismount()considerably slower. When I iterate over a big test dir tree of mine, with some Python code that has an-xdevoption where I then need to check every dir whether it's a mountpoint (and if so, not traverse it), I get for example the following results:realpath()one gets:without:
That's an extra of 556130
newfstatat()s because of therealpath(). I assume it calls a stat for each path and every path component, and if I count the path components via:the numbers match (off a few, because stuff gets written/deleted in
/home).Even after a lot of thinking, I don't quite understand why it would be needed.
The bug #46718 mentions describes it as fixing it for the case when there's some directory where the user doesn't have permissions, but wouldn't a better solution for that be to simply not catch the exception in:
cpython/Lib/posixpath.py
Lines 205 to 206 in e860e52
?
realpath()resolves symlinks, so I tried to construct a directory structure, where some symlinks would trickismount()without therealpath()into a false positive or false negative, but failed to do so:Consider e.g.
I copy&pasted
ismount()with the call torealpath()removed, and fromwithin
test/ran:just as it should.
Could it be, that something changed and the realpath() is no longer
needed?
Looking at the
lstat()for the parent path, gives the following:=> both on dev 28 (that's the device of test/)
so it seems as if
os.stat()would already resolve the symlinksWhereas:
=> the symlink to
..within the mountpoint, dev 78=> both, the mountpoint, dev 78
=> again, the plain dir outside the mountpoint, dev 28
So Do I miss anything here, what the call of
realpath()is really good for? Or could it be dropped?calestyo commentedon Aug 29, 2022
The point above in #96328 (comment) is bogus.... well partially.
The check for not-a-symlink is needed, though I still think one could perhaps improve it to check for the pathname being a true directory (as no other file types can be mountpoints, I'd guess).
An example where the function wouldn't work without the not-a-symlink check is:
then an
ismount("mnt/foo/rl")without the not-a-symlink check would giveTrue, because thelstat()onmnt/foo/rlwould give the symlink dev 78, whereas onmnt/foo/rl/..it would give/.So together with #96328 (comment) I'd say that it seems
stat()resolves all path components except the last (which is resolved or not, depending onfollow_symlinks.Does that make sense?
calestyo commentedon Aug 29, 2022
I might have also found a (questionable?) reason for the
realpath(). It seems it doesn't really have much to do with symbolic links and their resolution, but (as it was stated in the original bug) with permissions.Consider e.g. a directory
/foo/no-perms/, whereno-permsis not accessible, thenos.stat("/foo/no-perms/", follow_symlinks=False)succeeds, butos.stat("/foo/no-perms/..", follow_symlinks=False)fails with a permission error (I'd say: just as it should).The same with the
stat(1)tool.With
os.path.realpath():The seems IMO, at least the 3 latter, because Python cannot know what
testandbarlook like (they may be symlinks, or not exist at all).E.g. when
baris a symlink to/mnt, then as normal user:which is clearly wrong, but as root:
Likewise, the
realpath(1)tool (which I guess simply callsrealpath(3)C function?), as normal user, gives:Which I think is correct, unlike what Python does.
To simply collapsing
..should perhaps only work right "after" the inaccessible directory, and only when that is a inaccessible directory, not an inaccessible symlink (should that even be possible on some platforms).But now back to
ismount()… not sure whether this time I understood everything, but it seems that the call torealpath(), with - as said above - is IMO quite expensive, helps only in one single case, namelyno-perms/.., in which no-perms is a mountpoint candidate.It should not help if the mountpoint candidate is somewhere below
no-perms/or either because Python-realpath()’s results seem strange in these cases, or because it still couldn't ´stat()anything **below**no-perms/`.Now arguably, the first case is not so uncommon, but still the solution seems extremely expensive, especially because of the many
stat()s it may cause (on each component of the path, AFAIU).calestyo commentedon Aug 29, 2022
Maybe one could catch the
PermissionError: [Errno 13] Permission denied:…and only then userealpath()… and that perhaps better withstrict=True?calestyo commentedon Aug 30, 2022
I further thought about how one could get rid of the expensive
realpath()…Assuming we remove the
realpath()... (but leave the check for non-symlink):…/d1/d2…/d1/d2/..d1not possible…/d1/s2…/d1/s2/..d1possible (vias2), but non-symlink-check catches it…/s1/s2…/s1/s2/..s1points to somedx1, which containss2; break-out of thatdx1possible (vias2), but non-symlink-check catches…/s1/d2…/s1/d2/..s->dx1, thend2, is really indx1/(not ins1, which is symlink);…/s1/d2/..would give…/dx1, which is the true parent ofd2After the no-symlink-check, construct the parent path as removing the last pathname component, not using
/...The break-out cases from above should still be handled, as they're checked before.
Now for the case of
d2not being accessible by the user (which is what therealpath()was, AFAIU, previously used for), we assumed2respectively the target ofs2is not accessible by the user.Still a problem? No, because we don't have
non-accessible/..anymore in thelstat(), so that wont't fail.But do we still get the true parent?
…/d1/d2…/d1/d2d1is the true parent ofd2…/d1/s2…/d1/s2False, because of the non-symlink-check,... but even without:d1is the true parent ofs2…/s1/s2…/s1/s2False, because of the non-symlink-check…/s1/d2…/s1/d2s1(i.e.stat()notlstat()the parent), even if it first points to another symlink, then we eventually get somedx1, which should however be the true parent directory ofd2(Whether we have a trailing
/after the parent pathname, shouldn't matter, I think. If we keep a trailing/than we could probably uselstat(), because the symlinks would be still dereferenced, but I think that would make the code less readable.Now @pitrou wrote in #46718 (comment):
But that's anyway already ruled out because of the non-symlink-check.
And in #46718 (comment) he said:
I understand this as follows in my terms:
We check
ismount("/s1/d2")(=/A/B/),s1points (eventually) todx1, which is a mountpoint, which however has to truly containd2.So if
d2is not a mountpoint (in which case we'd of course want the different device ID), why should it have anotherst_devthandx1(it's a true child of it)? It might have another st_dev thats1(the symlink), but when we don'tlstat()the parent, butstat()it, that should be fine.TBH, even with the `path/..` way for getting the parent **and** using `lstat()`, I don't understand @pitrou’s above case (at least not the way as he wrote).
What would IMO happen is rather:
If `/A` is used as parent pathname and points to a mountpoint, then the `lstat()` would still get the device ID of the symlink. This may or may not be the same device ID as where `/A` points to and which is the true parent. So regardless of whether `/A/B` is a mountpont or not, the result might be right or wrong.
But back to my proposal:
Are the other cases ok (i.e. do we stat the right thing?), if we do
stat()(for the parent only) instead oflstat()?…/d1/d2…/d1/d2d1in the parent path is anyway a directory, so no following…/d1/s2…/d1/s2d1in the parent path is anyway a directory, so no following…/s1/s2…/s1/s2Falseout because of the non-symlink-check…/s1/d2…/s1/d2s1be followed, as this must be the true parent ofd2Not sure if I missed anything or messed up my case, but AFAIU, we should be able to drop the
realpath(), if:stat.S_ISDIR.then, I guess)stat()rather thanlstat()but for the parent onlyAny thoughts?
calestyo commentedon Aug 30, 2022
@eryksun
btw: I followed that a bit up, and while you're right with
chroot()the standard seem still to differ between the system root directory and a process’s root directory.So I'd say that the POSIX definition really means the true system root dir, and not any chroot’s root dir. And thus strictly (and practically) speaking, the check comparing the inode numbers is wrong.
calestyo commentedon Aug 31, 2022
More reason why using
path/..may at least in theory be problematic:AFAIU, POSIX does in principle not mandate for
..to exist.It says in 4.13 Pathname Resolution:
"may"
And in 3.144 Empty Directory:
"at most", IMO indicates that it could also lack them.
However, 4.13 Pathname Resolution says:
Not sure if on can interpret the "shall" in the sense of "it must be there" or just "if it's there, then it shall do.."
Of course, in practise this probably never happens unless there's some filesystem bug (which I've however actually already seen, i.e. directories where
..was "accidentally" missing).calestyo commentedon Aug 31, 2022
Also,
os.path.realpath()does more pretty strange thing, e.g. a path…/dir/regular-file/..wheredirandregular-filedo exist as such, results in…/dir, even withstrict=True, but that seems utterly wrong, asregular-filehas no..directory entry.For
…/dir/non-existent/..this only works withstrict=False.Thought, AFAICS, this strange property of
os.path.realpath()is not need foros.ismount("…/dir/regular-file")to give the correctFalse… I think that would also without work by already catching the exception.calestyo commentedon Nov 14, 2022
@eryksun Any expert ideas on my follow-up comments?
mxmlnkn commentedon Mar 29, 2024
Another situation that
ismountdoes not detect correctly is when the FUSE-providing process gets uncleanly killed or crashes.It seems like the
mountpointcommand has the same problem and without-qit prints the same error asls. The mount point is still visible inmount, so it should be possible to detect it.bionade24 commentedon Jun 26, 2025
Any progress on this and #81520 ? The current state is very misleading. The bind behaviour should also be in the docstring, not only the external doc.
calestyo commentedon Jun 26, 2025
Uhm, not really from my side. At least IMO it’s for sure not enough to deal with all the above by a mere documentation update.
Maybe some core developer could some day have a look at the findings from above regarding
realpath()and whether things could be improved there.That main problem remains to be "what is a mountpoint" and how to detect it. Of course one could stick to POSIX' definition but that's IMO simply ignoring reality (and delivering false results). So maybe it's time to make this platform-dependent.
But even then, assuming one would e.g. do that for Linux, it may become necessary to add some parameters to the function, which allow to control what should be considered a mountpoint.
E.g. are things like subvols considered mountpoints, if they're beneath another mountpoint from the same fs, like in
/btrfs-fs1-subvol-a/btrfs-fs2-subvol-a/it’s clear that there's a mountpoint (and evenst_devwill differ), but what about/btrfs-fs1-subvol-a/btrfs-fs1-subvol-b/or even/btrfs-fs1-subvol-a/btrfs-fs1-subvol-a/- for those I’d guess it depends on what the user wants (similar for bind-mounts, if it's the same fs).bionade24 commentedon Jun 28, 2025
If that's true, why does the
pydoccommand even exist? It should at least be contain such crucial deviations from expected behaviour.What's a mountpoint is clearly determined by the returncode of
mountpointfrom util-linux, which according to my tests always behaves as I'd expect it. I think the real question about the behaviour has already been written here or some thread about the same topic. Is a Linux-specific logic wanted or not.calestyo commentedon Jun 28, 2025
I haven’t said that one shouldn’t add documentation to at least warn about all this. But it wouldn’t fix the real issue.
Apart from that being Linux-only (as I've said above, this will likely have to become platform-dependent) even that doesn’t seem to formally define what a mountpoint is - it may be mere implementation details.
And "expectancy" (with respect to subvols, bind mounts, etc.) is IMO really dependent on the use case.