Latest qmake is now using unnamed temporary files and fails when running with pseudo. Error occcurs when calling linkat with the final name for the file (-1, No such file or directory) char path[PATH_MAX]; fd = open("/path/to/dir", O_TMPFILE | O_RDWR, S_IRUSR | S_IWUSR); snprintf(path, PATH_MAX, "/proc/self/fd/%d", fd); linkat(AT_FDCWD, path, AT_FDCWD, "/path/for/file", AT_SYMLINK_FOLLOW);
Okay that's horrible. linkat() already has AT_EMPTY_PATH for this, which would have been: linkat(fd, "", AT_FDCWD, "/path/for/file", AT_SYMLINK_FOLLOW | AT_EMPTY_PATH) But instead of using that, they're requesting a hard link to a file descriptor file in another filesystem. And then there's a hack where, since 2.6.18, AT_SYMLINK_FOLLOW on /procs/self/fd/N, that is silently turned into a way to create a link to an unlinked file. This will require some special-casing and we should probably handle both cases. But also I would like to state for the record that both of these are insanely bad interfaces. If you want a way to link a file descriptor, make a way to link a file descriptor. Instead, we get one thing that only sometimes under special circumstances changes the meaning of a parameter entirely, and also a case where you're allowed to specify that you want a hard link to another filesystem, but you specified to follow symbolic links, so that's okay then. Ugh.
(In reply to comment #1) > But also I would like to state for the record that both of these are > insanely bad interfaces. If you want a way to link a file descriptor, make a > way to link a file descriptor. Instead, we get one thing that only sometimes > under special circumstances changes the meaning of a parameter entirely, and > also a case where you're allowed to specify that you want a hard link to > another filesystem, but you specified to follow symbolic links, so that's > okay then. Ugh. I totally agree with you. linkat(fd, "", ..., AT_EMPTY_PATH) would have been a lot better, but that doesn't work. The man page for open(2) gives this incantation: O_TMPFILE must be specified with one of O_RDWR or O_WRONLY and, optionally, O_EXCL. If O_EXCL is not spec- ified, then linkat(2) can be used to link the temporary file into the filesystem, making it permanent, using code like the following: char path[PATH_MAX]; fd = open("/path/to/dir", O_TMPFILE | O_RDWR, S_IRUSR | S_IWUSR); /* File I/O on 'fd'... */ snprintf(path, PATH_MAX, "/proc/self/fd/%d", fd); linkat(AT_FDCWD, path, AT_FDCWD, "/path/for/file", AT_SYMLINK_FOLLOW);
Huh, I wonder *why* it doesn't work, since it's documented anyway. Or do you mean "doesn't work with pseudo"? Because neither of them will yet. I'm really swamped right now, I have three or four things to fix when I have a free day or two.
(In reply to comment #3) > Huh, I wonder *why* it doesn't work, since it's documented anyway. > > Or do you mean "doesn't work with pseudo"? Because neither of them will yet. > > I'm really swamped right now, I have three or four things to fix when I have > a free day or two. As far as we can tell, it doesn't work under pseudo. We found it while trying to compile Qt 5.10 with it, as part of building the Qt layer. We currently don't see a way to apply a workaround that would catch this reliably and would not affect the functionality for everyone else. At this point, the suggestion is for the Qt layer to patch the sources and disable the feature.
It's not very clear to me: is there now a workaround on the Qt side, or is it something that has to be solved on pseudo side?
There is a workaround applied to Qt in meta-qt5 layer, but there won't be anything in Qt itself.
The workaround is for open+O_TMPFILE failing, which happens if you run against an old kernel and happened also with the qemu bug. There's no workaround in the sources for linkat() failing. This is where pseudo is failing. The layer disables the O_TMPFILE feature completely until pseudo is fixed, but anyone not using the layer or somehow tinkering with it the wrong way will be affected.
It appears the questions have been answered. If you still need more information, place it back in NEEDINFO and can you restate/clearify the question?
Created attachment 4042 [details] Alex's naive patch Here's a naive patch I tried to see if it helps, and looks like it doesn't. TBH, I have no real idea what I did here, but if anyone wants to comment, please do.
Assigning to Seebs; at this point I would really need guidance on the patch, how this should work and how to move forward.
Hi, sorry for the delay, stuff has pretty much just been Staying On Fire for a long time. I'm travelling this week but should be able to look more closely at this when I get back. I think a correct solution for this really weird use case is going to have to be hard-coding some kind of awareness of that special path structure into linkat(), to have it detect that the path it's been requested to link is a magic path that is actually really a file descriptor, not a path at all, so just create a new link using the information we have for that file descriptor.
I don't think it's special with linkat(). The /proc/PID/fd path *is* the actual file descriptor, regardless of whether the file the readlink() returns exists or not. The path it shows is the path that was used to open, but that doesn't need to be visible to you.
(In reply to comment #11) > Hi, sorry for the delay, stuff has pretty much just been Staying On Fire for > a long time. I'm travelling this week but should be able to look more > closely at this when I get back. > > I think a correct solution for this really weird use case is going to have > to be hard-coding some kind of awareness of that special path structure into > linkat(), to have it detect that the path it's been requested to link is a > magic path that is actually really a file descriptor, not a path at all, so > just create a new link using the information we have for that file > descriptor. That's what I attempted to do in the patch I attached, but it doesn't seem to help with building qt5. Like I said, I don't really know what I'm doing in the patch, so someone competent needs to look at it :)
huh, that's odd. I had a few minutes, so I built a trivial test case so I could verify my understanding of what's wrong. And... it worked? #include <errno.h> #include <fcntl.h> #include <unistd.h> #include <stdio.h> #include <stdlib.h> int main(void) { char buffer[256]; int fd = open("a", O_RDWR); snprintf(buffer, 256, "/proc/self/fd/%d", fd); int rc = linkat(AT_FDCWD, buffer, AT_FDCWD, "b", AT_SYMLINK_FOLLOW); fprintf(stderr, "rc: %d, errno: %d\n", rc, errno); exit(0); } Given this, if I make a file a, and run this, I end up with a hard link to it, b, with the same stats, and linkat succeeds. Is this happening while running in a chroot environment or something, so that /proc/self/fd wouldn't actually exist?
For the kernel, the file actually exists, so linkat() will work. However, the /proc/self/fd/N link may be broken if you readlink() it. That's actually the case for O_TMPFILE: it always shows up as deleted. So if you readlink(), you'll get garbage. You need to pass the actual /proc/self/fd/N name to the kernel for the operation to complete. Try with O_TMPFILE or by inserting unlink("a") before the linkat(): #define _GNU_SOURCE 1 #include <errno.h> #include <fcntl.h> #include <unistd.h> #include <stdio.h> #include <stdlib.h> int main(void) { char buffer[256]; int fd = open(".", O_RDWR | O_TMPFILE, 0666); snprintf(buffer, 256, "/proc/self/fd/%d", fd); int rc = linkat(AT_FDCWD, buffer, AT_FDCWD, "b", AT_SYMLINK_FOLLOW); fprintf(stderr, "rc: %d, errno: %d\n", rc, errno); exit(0); }
I got this half-fixed, in that this case works, but it breaks things horribly now if you use O_TMPFILE and *don't* link the file, because then we have a stray entry in the database with no path, and a dev/ino pair that could be reused. So this will take a bit longer. I badly underestimated this one. It looks like I have most of the hooks in place for the thing, though; I need a table of file info parallel to the fd_paths table that indicates files which should be deleted on close, probably. So, open with O_TMPFILE will send an initial CREAT message, then a MAY_UNLINK, and then it can send DID_UNLINK if it needs to. ... Still some loose ends here. I'll try to get this fixed up "soon".
Workaround: reject O_TMPFILE and return either EOPNOTSUPP or EISDIR. The caller will fall back to a regular file with a temporary name.
Okay, so, I pushed a patch that does make this work, but it's *awful*. I do not recommend it. I admit that the ENOTSUPP answer is tempting. But I think I may be able to do a better job by just abandoning any pretense that this is really "linking" a file in any meaningful sense. But also I'm tired and I'm not gonna try to fix it tonight. But the basic idea would be: Revert the top two patches in master, and redo the O_TMPFILE support patch in a completely different way, where we just send an O_CREAT for the "linked" file when linking. (Also, probably, change the behavior with O_TMPFILE to suppress creating any database entry, and definitely not try to use the directory name as the path of the opened file, since that's clearly wrong.)
okay, sorry for the delay, this was horrible and i had to retry it a bit to make a patch i didn't hate. now in master, along with several other fixes, as pseudo 1.9.0. (the 1.9 version bump is for epoll.)
(In reply to comment #19) > okay, sorry for the delay, this was horrible and i had to retry it a bit to > make a patch i didn't hate. now in master, along with several other fixes, > as pseudo 1.9.0. (the 1.9 version bump is for epoll.) Can you upload the 1.9.0 tarball here please? http://downloads.yoctoproject.org/releases/pseudo/
I don't think I actually have access to the upload dir. can someone else grab this and put it in the right place? https://www.seebs.net/tmp/pseudo-1.9.0.tar.bz2 md5 7ec8409bccdbd9c958367469b6059071 sha256 a3f150cc55b2fa009c9411335981b55675ead4e5cb4a9c36ed40b32f81d020c1 this includes two more patches I'd missed, one is restoring higher reliability (fixing a subtle bug introduced by FASTOP), the other is a fix for openat flags because of a silly typo.
Unfortunately, I'm seeing the following error with the latest upstream commit of pseudo: | DEBUG: Executing shell function do_install | got *at() syscall for unknown directory, fd 4 | unknown base path for fd 4, path usr | couldn't allocate absolute path for 'usr'. | chown: cannot access '/home/ak/development/poky/build/tmp/work/all-poky-linux/tzdata/2018c-r0/image/usr': Bad address | got *at() syscall for unknown directory, fd 4 | unknown base path for fd 4, path etc | couldn't allocate absolute path for 'etc'. | chown: cannot access '/home/ak/development/poky/build/tmp/work/all-poky-linux/tzdata/2018c-r0/image/etc': Bad address | WARNING: exit code 1 from a shell command. I've bisected between that and PSEUDO_1_8_2, and established the the commit that introduces the failure is indeed this one: http://git.yoctoproject.org/cgit/cgit.cgi/pseudo/commit/?id=bbebd1e85f4b3f30a355e8721052c568e8408cfc (handle O_TMPFILE and linkat()) Also, latest pseudo locks up altogether when epoll is enabled, but let's address issues in order :)
Gah. Thanks for the report, sorry for delay in getting back to you on it. Do you have a reproducer smaller than "build yocto" available? I can try to look, but I don't know how to trigger this, and a complete yocto build is Large.
I don't know how to trigger this outside of yocto either. But you don't need a complete yocto build - this happens as soon as the first target recipe is reaching do_install(). I just checked that on my machine with an empty sstate cache this takes about three minutes (when running 'bitbake os-release' for example, which is an arch-independent recipe without dependencies, and so does not require building the toolchain or libc).
Ah, the branch that you can take is here: http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/log/?h=akanavin/pseudo-1.9.0
I can also enable additional logging, and we can try to find where things diverge in the working and failing cases, but I'd need guidance from you on what to enable.
The problem appears to be that O_TMPFILE has bits in it that are also in other modes, so (flags & O_TMPFILE) was the wrong test. Fixed now in upstream master.
O_TMPFILE has the bits to O_DIRECTORY
Thanks, that does seem to fix the issue! Also, epoll hanging problem seems to be gone, looks like it was the same problem. I've posted the update patch to oe-core list for review and autobuilder testing: http://lists.openembedded.org/pipermail/openembedded-core/2018-February/147507.html Seebs, I'd like to ask two more things, neither is complicated :) If you could: a) look at remaining patches, and see what can be taken to upstream git? http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/tree/meta/recipes-devtools/pseudo/files?h=akanavin/pseudo-1.9.0 b) then tag the latest upstream commit as, for example, 'pseudo-1.9.1'. I'd really prefer that we start using git tags to map versions to specific commits. Thanks :)
@Samuli @Thiago Can you please try this poky branch and see if a) it generally works; b) it fixes the specific issue that started this bug? http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/log/?h=akanavin/pseudo-1.9.0
Assigning to Samuli - please do test the fix; we do not want to merge it to oe-core and then find out it does not actually help with the issue that started this bug.
verified that open(O_TMPFILE)+linkat now works with qmake, as far as files are copied. However, there are always random files that fail the host-user-contaminated QA check during packaging.
We're tracking the host user contamination issue elsewhere: bug 12434, bug 11299. I'll send the patch to oe-core list again, so hopefully it makes it in before 2.5
The patch is in oe-core/poky master: http://git.yoctoproject.org/cgit/cgit.cgi/poky/commit/?id=d50b9c511e17c6f582ef39b61eaaa8d4555eabba