13288 2019-04-17 23:28:53 +0000 pseudo should not follow symlinks in /proc 2026-05-14 15:16:11 +0000 1 1 6 Yocto Project Subprojects Pseudo pseudo master x86 Multiple RESOLVED FIXED Medium+ normal 5.99 1 matt.cowell unassigned david.nystrom paul randy.macleod sam seebs tgamblin yp.pseudo.watcher yp.watcher --- --- No (bug/feature does not impact docs) oldest_to_newest 83573 0 4480 matt.cowell 2019-04-17 23:28:53 +0000 Created attachment 4480 patch to fix issue Some symlinks in /proc, such as those under /proc/[pid]/fd, /proc/[pid]/cwd, and /proc/[pid]/exe that are not real and should not have readlink called on them. These look like symlinks, but behave like hardlinks. Readlink does not return actual paths. Previously pseudo_fix_path would expand files such as /dev/stdin to paths such as /proc/6680/fd/pipe:[1270830076] which do not exist. This issue affects: - deleted files - deleted directories - fifos - sockets - anon_inodes (epoll, eventfd, inotify, signalfd, timerfd, etc) The proposed fix is to check the filesystem type of symlinks before calling readlink(), and skip symlink expansion for that element of the path if the filesystem type is procfs. Reproduction information: Preconditions/Environment ------------------------- run a shell under pseudo Triggering Action/Cause ----------------------- -> echo foo | cat /dev/stdin Expectation ----------- "foo" prints Actual Result ------------- "cat: /dev/stdin: No such file or directory" prints Reproducibility --------------- 100% Same test case with debug enabled: Before fix: -> echo foo | PSEUDO_DEBUG=pV cat /dev/stdin paes: newpath , element list </dev/stdin> element to add: '' pae: '', + '', is_dir 1 element to add: 'dev' pae: '', + 'dev', is_dir 1 element to add: 'stdin' pae: '/dev', + 'stdin', is_dir 1 paes: newpath , element list </proc/self/fd/0> element to add: '' pae: '', + '', is_dir 1 element to add: 'proc' pae: '', + 'proc', is_dir 1 element to add: 'self' pae: '/proc', + 'self', is_dir 1 paes: newpath /proc, element list <6680> element to add: '6680' pae: '/proc', + '6680', is_dir 1 element to add: 'fd' pae: '/proc/6680', + 'fd', is_dir 1 element to add: '0' pae: '/proc/6680/fd', + '0', is_dir 1 paes: newpath /proc/6680/fd, element list <pipe:[1270830076]> element to add: 'pipe:[1270830076]' pae: '/proc/6680/fd', + 'pipe:[1270830076]', is_dir 1 <nil> + /dev/stdin => </proc/6680/fd/pipe:[1270830076]> base_path[follow]: <nil></>/dev/stdin => /proc/6680/fd/pipe:[1270830076] cat: /dev/stdin: No such file or directory After fix: -> echo foo | PSEUDO_DEBUG=pV cat /dev/stdin paes: newpath , element list </dev/stdin> element to add: '' pae: '', + '', is_dir 1 element to add: 'dev' pae: '', + 'dev', is_dir 1 element to add: 'stdin' pae: '/dev', + 'stdin', is_dir 1 paes: newpath , element list </proc/self/fd/0> element to add: '' pae: '', + '', is_dir 1 element to add: 'proc' pae: '', + 'proc', is_dir 1 element to add: 'self' pae: '/proc', + 'self', is_dir 1 pae: '/proc/self' is procfs symlink, not expanding element to add: 'fd' pae: '/proc/self', + 'fd', is_dir 0 element to add: '0' pae: '/proc/self/fd', + '0', is_dir 1 pae: '/proc/self/fd/0' is procfs symlink, not expanding <nil> + /dev/stdin => </proc/self/fd/0> base_path[follow]: <nil></>/dev/stdin => /proc/self/fd/0 foo Other test cases that can reproduce: - Using bash process substitution: cat <(echo foo) - Accessing a deleted file under /proc/[pid]/fd/* , /proc/[pid]/exe, or similar - possibly glibc APIs which use symlinks under /proc (there are several including fexecve). 84688 1 seebs 2019-08-15 15:44:20 +0000 *** Bug 13483 has been marked as a duplicate of this bug. *** 84694 2 sam 2019-08-15 17:22:40 +0000 I like this solution and patch. I don't know everything intended to achieved by pseudo following symlinks, or how much of that is relevant to /proc, but if there is a compulsion to follow the symlinks for proc, then maybe: instead of never following a symlink on procfs, we could just avoid following the symlink whose lstat failed if it is on a procfs so: lstat("/proc/6724/fd/63", {st_mode=S_IFLNK|0500, st_size=64, ...}) = 0 readlink("/proc/6724/fd/63", "pipe:[21346885]", 4096) = 15 lstat("/proc/6724/fd/pipe:[21346885]", 0x7ffe0f16e040) = -1 ENOENT (No such file or directory) As the final lstat failed, use the pre-readlink value for open, the pre-readlink file was on a procfs 84700 3 matt.cowell 2019-08-15 17:51:57 +0000 > we could just avoid following the symlink whose lstat failed I had considered this, but there are cases where the lstat in procfs can improperly succeed and dereference the wrong path, such as when namespaces are in use or if a file with the suffix " (deleted)" exists (in the case of deleted files / directories). Mount namespace example (two processes in different namespaces): sh-5.0# readlink /proc/27570/cwd /proc/27574/cwd /root/foo /root/foo sh-5.0# tail /proc/27570/cwd/asdf /proc/27574/cwd/asdf ==> /proc/27570/cwd/asdf <== ns1 ==> /proc/27574/cwd/asdf <== ns2 Deleted file example: sh-5.0# echo right file > bar sh-5.0# echo wrong file > 'bar (deleted)' sh-5.0# exec 200<>bar sh-5.0# rm bar sh-5.0# realpath /proc/self/fd/200 /root/bar (deleted) sh-5.0# cat /proc/self/fd/200 right file sh-5.0# cat "$(realpath /proc/self/fd/200)" wrong file 86051 4 randy.macleod 2020-01-16 16:13:39 +0000 Moved to M3 but the earlier we get pseudo patches in the better. 87936 5 randy.macleod 2020-08-13 10:11:28 +0000 Sakib, Peter thinks that the patch is okay. He's a bit concerned about the cost of checking that for every single symlink path element, but he said that he can't think of a good way to avoid it. Please prepare a patch and on an isolated system that only you use, time builds before and after to see if there is a measureable difference. If there isn't submit the patch and CC seebs. Thanks. 89719 6 randy.macleod 2021-03-24 00:23:28 +0000 ping. 3.3-M4 or has this been merged? 89832 7 randy.macleod 2021-03-31 00:10:48 +0000 ping. 3.3-M4 or has this been merged? 89856 8 randy.macleod 2021-04-01 16:27:34 +0000 Medium+ needs a milestone. 91469 9 sam 2021-09-18 09:27:54 +0000 As it's nearly 30 months since a patch has been supplied. Instead of thinking about merging for years on end, please could you merge and then spend years thinking about reverting the merge for the rest of time? 91474 10 sakib.sajal 2021-09-20 13:42:58 +0000 If its so important for you, do it yourself. Cheers! :) 91477 11 sam 2021-09-20 18:18:26 +0000 What kind of response is that? I'm running the fix I posted in duplicate https://bugzilla.yoctoproject.org/show_bug.cgi?id=13483 so don't need the fix merging. I'm thinking of all your other users for whom /dev/fd/2 etc hasn't been working for *years* You've time for inane disrespect for people who take the time to report bugs *and* post fixes but no time to help users when you have the fix on a plate. As far as I can tell this bug languishes because correct behaviour might take longer than the incorrect behaviour that causes total failure for simple code that ought to work. You think it's not that important! 91478 12 sakib.sajal 2021-09-20 18:45:00 +0000 My apologies if my comment seemed inappropriate/disrespectful. I have been trying to find time to work on upstream yocto bugs but have not been able to do so. I will get on them early October. Thank you! 92860 13 randy.macleod 2022-03-24 15:56:18 +0000 We added the patch and it seemed to mostly work okay but there is at least one use case, mentioned in the revert log, that failed so the patch was reverted: https://git.yoctoproject.org/pseudo/commit/?id=df1d1321fb093283485c387e3c933d2d264e509c Early in the next release, we need to understand why this happened and decide if we want to add one or more special exceptions to the patch. 92912 14 sam 2022-04-01 08:00:48 +0000 I suppose it failed because /proc/self/fd/3 is obviously reported by the kernel as a symlink to /home/pokybuild/yocto-worker/beaglebone/build/build/tmp/work/beaglebone_yocto-poky-linux-gnueabi/core-image-minimal/1.0-r0/sdk-ext/image/usr/local/oe-sdk-hardcoded-buildpath/layers/build/meta/ and that resolved path was needed by pseudo in order to perform more file system mappings, and therefore we cannot simply refuse to resolve /proc symlinks However, the basis of the reverted patch is that we can't always safely resolve /proc symlinks. But maybe we can... Certainly under /proc, opening a symlink is not the same as opening readlink(symlink) because for diagnostic reasons, the kernel emits bogus symlink target which the kernel itself does not follow when opening. Apparently, only the kernel can properly resolve symlinks under proc -- it knows when the final stage symlink is bogus. (This problem is not restricted to pseudo; pre-use readlink resolution simply cannot be trusted in /proc by any process). But can we tell after all? And in a simple way? Just as surely as we need to not fail when we fail to resolve /dev/*/fd/* to pipe:[...], we also need a way to resolve /proc symlinks that do direct back out of the /proc file system but which may have a deleted target I had supposed in comment 2 [https://bugzilla.yoctoproject.org/show_bug.cgi?id=13288#c2] that the last stage readlink would always fail when the last stage was bogus: /proc/6724/fd/63 -> pipe:[21346885] /proc/6724/fd/64 -> /tmp/file (deleted) But it is evident from the examples in comment 3 [https://bugzilla.yoctoproject.org/show_bug.cgi?id=13288#c3] that real clashing files can exist with those names. What makes the difference which I failed to take into account before, is whether or not the link target is absolute, meaning it begins with a / Real clashing/deleted files can only exist for those names if the symlink target begins with a / If the link target does NOT begin with a / then it will not leave the /proc file system and we can safely do as I proposed in comment 2 which is: > instead of never following a symlink on procfs, we could just avoid > following the symlink whose lstat failed if it is on a procfs adding the condition: provided that it was not an absolute symlink This way /dev/fd/3 will resolve to /proc/self/fd/3 to /proc/6724/fd/3 and stop before resolving to pipe:[21346885] whose lstat will fail. The ultimate filename /proc/6724/fd/3 will work But if the proc symlink target were an absolute path then we have additional conditions to follow, because the issue is how to safely navigate to that exiting link (/proc/*/fd/3 -> /home/pokybuild/...) in a way that won't choke on " (deleted)" or cross-namespace links. The absolute symlink in /proc is not like a symlink in a pseudo chroot, or a normal filesystem. It seems to refer to real final paths with any symlinks already resolved, and no need for any chroot path fixups before further symlink resolution (unlike symlinks in the chroot directories on a normal non-proc file system). So not only must the lstat of the symlink target not fail, but the st_dev and st_ino properties must match those of a stat of the symlink, and thus we know that the symlink target exists and is the original valid target and not a new one with the same name. Therefore (if it is likely to matter in real life) we can tell whether " (deleted)" is part of the name or not, whether or not the target symlink is valid. If they match, then the symlink target is valid however many times " (deleted)" may appear in it due to tricksie hobbitses, and thus we have the link target that we can continue resolving, having successfully navigated outside of the /proc filesystem Note: there are absolute symlinks in /proc which refer back to /proc, e.g.: * /proc/1/task/1/fd/10 -> /proc/1/mountinfo * /proc/482/fd/11 -> /proc/sys/kernel/hostname but that won't be a problem with these methods, /proc links just continue to get processed as /proc as described above. Does this seem reasonable? Cross namespace links Although these are mentioned in comment #3 I don't know how much of real a issue this is. But because absolute links are resolved, you won't easily find anything like /proc/2345/fd/5 -> /proc/1234/fd/2 Even if you try to open the fd of another process with 5>/proc/1234/fd/2 ultimately /proc/2345/fd/5 will point to whatever /proc/1234/fd/2 is pointing to. There are absolute symlinks in /proc which may back to another related process, e.g.: /proc/4170249/root -> /proc/2527499/fdinfo\ (deleted) /proc/4170249/task/4177332/fd/37 -> /proc/4170249/status /proc/1337266/fd/8 -> /proc/1337266/mountinfo but I'm not certain they can be an issue. Maybe we can refuse to traverse links from /proc/X/ to /proc/Y/ where X != Y Maybe where they aren't part of the same process group? Or session? Or mount namespace? Or children of a common pseudo invocation? Do we expect such symlinks to occur in /proc where the target also needs some path massaging to avoid an abort? Where the target despite being in a different namespace is in a pseudo chroot path that this instance of pseudo is controlling and can validly massage? Surely if it's possible it's much less likely than the situation that reverted Matt's patch or the bug fixed by the Matt's patch, and it can be addressed when it happens. (I wish I had kept my original patch now, and I see I didn't attach it to #13483) Simply put quit processing without error for symlinks anywhere whose target in /proc fails lstat rather than symlink sources in /proc whose target fails lstat In /proc if lstat target succeeds and (target is not in /proc) then we add the st_ino st_dev test to confirm step by step: * resolve the symlink to the fully qualified target * if lstat(target) fails * and the target is in /proc then return the symlink not the target and let the kernel resolve it on use (it will be pipe:[...] or something like that * if st_no and st_dev of stat(symlink) don't match lstat of target then return the target knowing it will fail on use * continue processing based on the new target as input Sam 95804 15 randy.macleod 2023-06-22 15:05:31 +0000 Sakib doesn't have time to continue to test this since it failed some YP autobuilder tests. -- YP bug review 96454 16 tgamblin 2023-09-14 15:56:32 +0000 Moved to M4. 97058 17 randy.macleod 2023-10-30 15:29:09 +0000 Bulk move to 5.0 M1. -- Randy 97991 18 randy.macleod 2024-01-25 14:28:45 +0000 Bulk move of bugs owned by "unassigned" from 5.0-M2 to 5.0-M3. 98309 19 randy.macleod 2024-02-22 19:53:25 +0000 Bulk move of bugs from 5.0-M3 to 5.99. The bugs in this list are all unassigned and are ones that I consider to be nice to do some time but not essential track from milestone to milestone in 5.0 or 5.1. We agreed on this bug handling approach in the YP bug review on Feb 22, 2024. If you disagree, please move back to a 5.[012...]-M* milestone. 105422 20 paul 2026-05-13 13:26:28 +0000 Is this resolved by commit 814e554564b2 ("pseudo_util.c: Skip realpath like expansion for /proc on Linux") in pseudo? 105443 21 paul 2026-05-14 15:16:11 +0000 The issue was fixed as far as we could with the patch mentioned in my last comment, this doesn't exactly match the proposed fix described in earlier comments but it's the best we can reasonably do. 4480 2019-04-17 23:28:53 +0000 2019-04-17 23:28:53 +0000 patch to fix issue 0001-pseudo_fix_path-do-not-expand-symlinks-in-proc.patch application/octet-stream 2274 matt.cowell RnJvbSBjYjY5MTQ2ZDE4MzgyZjI3MmUzNDczMzhjZmM1YTg5MzBmNjkzZWQ2IE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBNYXR0IENvd2VsbCA8bWF0dC5jb3dlbGxAbm9raWEuY29tPgpE YXRlOiBXZWQsIDE3IEFwciAyMDE5IDE1OjUxOjA3IC0wNTAwClN1YmplY3Q6IFtQQVRDSF0gcHNl dWRvX2ZpeF9wYXRoOiBkbyBub3QgZXhwYW5kIHN5bWxpbmtzIGluIC9wcm9jCgpTb21lIHN5bWxp bmtzIGluIC9wcm9jLCBzdWNoIGFzIHRob3NlIHVuZGVyIC9wcm9jL1twaWRdL2ZkLAovcHJvYy9b cGlkXS9jd2QsIGFuZCAvcHJvYy9bcGlkXS9leGUgdGhhdCBhcmUgbm90IHJlYWwgYW5kIHNob3Vs ZCBub3QKaGF2ZSByZWFkbGluayBjYWxsZWQgb24gdGhlbS4gIFRoZXNlIGxvb2sgbGlrZSBzeW1s aW5rcywgYnV0IGJlaGF2ZSBsaWtlCmhhcmRsaW5rcy4gIFJlYWRsaW5rIGRvZXMgbm90IHJldHVy biBhY3R1YWwgcGF0aHMuICBQcmV2aW91c2x5CnBzZXVkb19maXhfcGF0aCB3b3VsZCBleHBhbmQg ZmlsZXMgc3VjaCBhcyAvZGV2L3N0ZGluIHRvIHBhdGhzIHN1Y2ggYXMKL3Byb2MvNjY4MC9mZC9w aXBlOlsxMjcwODMwMDc2XSB3aGljaCBkbyBub3QgZXhpc3QuCgpUaGlzIGlzc3VlIGFmZmVjdHM6 Ci0gZGVsZXRlZCBmaWxlcwotIGRlbGV0ZWQgZGlyZWN0b3JpZXMKLSBmaWZvcwotIHNvY2tldHMK LSBhbm9uX2lub2RlcyAoZXBvbGwsIGV2ZW50ZmQsIGlub3RpZnksIHNpZ25hbGZkLCB0aW1lcmZk LCBldGMpCi0tLQogcHNldWRvX3V0aWwuYyB8IDI3ICsrKysrKysrKysrKysrKysrKysrKysrKysr KwogMSBmaWxlIGNoYW5nZWQsIDI3IGluc2VydGlvbnMoKykKCmRpZmYgLS1naXQgYS9wc2V1ZG9f dXRpbC5jIGIvcHNldWRvX3V0aWwuYwppbmRleCBiNDY0MTdkMzIzYjAuLmNlMGE1NzExMWZiNSAx MDA2NDQKLS0tIGEvcHNldWRvX3V0aWwuYworKysgYi9wc2V1ZG9fdXRpbC5jCkBAIC0zMiw2ICsz Miw4IEBACiAjaW5jbHVkZSA8c3lzL3RpbWUuaD4KICNpbmNsdWRlIDx1bmlzdGQuaD4KICNpbmNs dWRlIDxsaW1pdHMuaD4KKyNpbmNsdWRlIDxzeXMvdmZzLmg+CisjaW5jbHVkZSA8bGludXgvbWFn aWMuaD4KIAogLyogc2VlIHRoZSBjb21tZW50cyBiZWxvdyBhYm91dCAoKnJlYWxfcmVnY29tcCko KSAqLwogI2luY2x1ZGUgPGRsZmNuLmg+CkBAIC00MCw2ICs0MiwxMSBAQAogI2luY2x1ZGUgInBz ZXVkb19pcGMuaCIKICNpbmNsdWRlICJwc2V1ZG9fZGIuaCIKIAorLyogT19QQVRIIGlzIGRlZmlu ZWQgaW4gZ2xpYmMgMi4xNiBhbmQgbGF0ZXIgb25seSAqLworI2lmbmRlZiBPX1BBVEgKKyNkZWZp bmUgT19QQVRIICAgICAgICAgIDAxMDAwMDAwMAorI2VuZGlmCisKIHN0cnVjdCBwc2V1ZG9fdmFy aWFibGVzIHsKIAljaGFyICprZXk7CiAJc2l6ZV90IGtleV9sZW47CkBAIC02ODgsNiArNjk1LDI2 IEBAIHBzZXVkb19hcHBlbmRfZWxlbWVudChjaGFyICpuZXdwYXRoLCBjaGFyICpyb290LCBzaXpl X3QgYWxsb2NhdGVkLCBjaGFyICoqcGN1cnJlCiAJICovCiAJaWYgKCFsZWF2ZV90aGlzICYmIGlz X2RpcikgewogCQlpbnQgaXNfbGluayA9IFNfSVNMTksoYnVmLT5zdF9tb2RlKTsKKworCQkvKiBk byBub3QgZXhwYW5kIHN5bWxpbmtzIGluIHRoZSBwcm9jIGZpbGVzeXN0ZW0sIHNpbmNlIHRoZXkg bWF5IG5vdCBiZSByZWFsICovCisJCWlmIChpc19saW5rKSB7CisJCQlzdHJ1Y3Qgc3RhdGZzIHNm czsKKwkJCWludCBmZDsKKworCQkJLyogc3RhdGZzIGZvbGxvd3Mgc3ltbGlua3MsIHNvIHVzZSBm c3RhdGZzICovCisJCQlmZCA9IG9wZW4obmV3cGF0aCwgT19DTE9FWEVDIHwgT19QQVRIIHwgT19O T0ZPTExPVyk7CisJCQlpZiAoLTEgIT0gZmQpIHsKKwkJCQlpZiAoMCA9PSBmc3RhdGZzKGZkLCAm c2ZzKSAmJiBzZnMuZl90eXBlID09IFBST0NfU1VQRVJfTUFHSUMpIHsKKwkJCQkJcHNldWRvX2Rl YnVnKFBEQkdGX1BBVEggfCBQREJHRl9WRVJCT1NFLAorCQkJCQkJInBhZTogJyVzJyBpcyBwcm9j ZnMgc3ltbGluaywgbm90IGV4cGFuZGluZ1xuIiwKKwkJCQkJCW5ld3BhdGgpOworCQkJCQlpc19s aW5rID0gMDsKKwkJCQl9CisKKwkJCQljbG9zZShmZCk7CisJCQl9CisJCX0KKwogCQlpZiAobGlu a19yZWN1cnNpb24gPj0gUFNFVURPX01BWF9MSU5LX1JFQ1VSU0lPTiAmJiBpc19saW5rKSB7CiAJ CQlwc2V1ZG9fZGlhZygibGluayByZWN1cnNpb24gdG9vIGRlZXAsIG5vdCBleHBhbmRpbmcgcGF0 aCAnJXMnLlxuIiwgbmV3cGF0aCk7CiAJCQlpc19saW5rID0gMDsKLS0gCjIuMTcuMQoK