Re: [OE-core] [PATCH RFC] bitbake.conf/pseudo: Switch from exclusion list to inclusion list
On Tue, 12 Dec 2023 12:44:47 -0600 "Mark Hatle" wrote: > /tmp - used by the compiler (and tons of other tooling) for temporary > files, we need this for sure. If memory serves, this is basically why it was an IGNORE path list originally. If we ever accessed anything outside of pseudo, we'd get leakage. The basic pattern looks like: * file gets created outside of our workspace * we then copy it in using something that tries to preserve ownership * since file wasn't being tracked through pseudo, it has real UID on it * now we've copied something into our workspace using that UID The very early design, back in our pre-Yocto build system, imagined a single unified database being used for the entire build process, persistently. That was maybe not the best design idea, but it was how we'd been using fakeroot and I didn't really revisit it at the time. Also, we had a *lot* of cross-pollination between components, so things would end up copying in or referring to files that were part of another package, without using the intermediate archived form, so... Wild times. Definitely one of those things where, with the wisdom of hindsight, I know enough about the problem that if you asked me to do it today I'd probably confidently tell you that it's not possible. :) -s -=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#192244): https://lists.openembedded.org/g/openembedded-core/message/192244 Mute This Topic: https://lists.openembedded.org/mt/103113368/21656 Group Owner: openembedded-core+ow...@lists.openembedded.org Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [OE-core] [PATCH][pseudo] Move __*xstat* and __xmknod functions to new subport 'old__x'
On Tue, 30 May 2023 19:54:41 -0700 Mark Hatle wrote: > - int existed = (real___xstat64(_STAT_VER, path, ) != -1); > + int existed = (base_stat64(path, ) != -1); Honestly, with the benefit of hindsight, I actually can't even think why I ever thought I should be using those directly instead of through the base_foo wrappers. Probably for a reason. ... Just in case, definitely don't merge this before testing it in case there's some insane reason for which the base_* wrappers didn't work here. > rc = real_fopen64(path, mode); > save_errno = errno; > @@ -20,7 +20,7 @@ > int fd = fileno(rc); > > pseudo_debug(PDBGF_FILE, "fopen64 '%s': fd %d %p>\n", path, fd, (void *) rc); > - if (real___fxstat64(_STAT_VER, fd, ) != -1) { > + if (base_fstat64(fd, ) != -1) { > if (!existed) { > real_fchmod(fd, PSEUDO_FS_MODE(0666 > & ~pseudo_umask, 0)); pseudo_client_op(OP_CREAT, 0, -1, -1, path, > ); diff --git a/ports/linux/guts/freopen64.c > b/ports/linux/guts/freopen64.c index 5fc9073..9bcc06a 100644 > --- a/ports/linux/guts/freopen64.c > +++ b/ports/linux/guts/freopen64.c > @@ -10,7 +10,7 @@ > */ > struct stat64 buf; > int save_errno; > - int existed = (real___xstat64(_STAT_VER, path, ) != -1); > + int existed = (base_stat64(path, ) != -1); > > rc = real_freopen64(path, mode, stream); > save_errno = errno; > @@ -19,7 +19,7 @@ > int fd = fileno(rc); > > pseudo_debug(PDBGF_FILE, "freopen64 '%s': fd %d\n", > path, fd); > - if (real___fxstat64(_STAT_VER, fd, ) != -1) { > + if (base_fstat64(fd, ) != -1) { > if (!existed) { > real_fchmod(fd, PSEUDO_FS_MODE(0666 > & ~pseudo_umask, 0)); pseudo_client_op(OP_CREAT, 0, -1, -1, path, > ); diff --git a/ports/linux/guts/fstat.c > b/ports/linux/guts/fstat.c index b089b15..80933e2 100644 > --- a/ports/linux/guts/fstat.c > +++ b/ports/linux/guts/fstat.c > @@ -8,7 +8,13 @@ > * int rc = -1; > */ > > - rc = wrap___fxstat(_STAT_VER, fd, buf); > + struct stat64 buf64; > + /* populate buffer with complete data */ > + real_fstat(fd, buf); > + /* obtain fake data */ > + rc = wrap_fstat64(fd, ); > + /* overwrite */ > + pseudo_stat32_from64(buf, ); > > /* return rc; > * } > diff --git a/ports/linux/guts/fstat64.c b/ports/linux/guts/fstat64.c > index 6dd97da..22d46a9 100644 > --- a/ports/linux/guts/fstat64.c > +++ b/ports/linux/guts/fstat64.c > @@ -8,8 +8,20 @@ > * int rc = -1; > */ > > - rc = wrap___fxstat64(_STAT_VER, fd, buf); > + pseudo_msg_t *msg; > + int save_errno; > > + rc = real_fstat64(fd, buf); > + save_errno = errno; > + if (rc == -1) { > + return rc; > + } > + msg = pseudo_client_op(OP_FSTAT, 0, fd, -1, 0, buf); > + if (msg && msg->result == RESULT_SUCCEED) { > + pseudo_stat_msg(buf, msg); > + } > + > + errno = save_errno; > /* return rc; > * } > */ > diff --git a/ports/linux/guts/fstatat.c b/ports/linux/guts/fstatat.c > index 3267641..7b9652d 100644 > --- a/ports/linux/guts/fstatat.c > +++ b/ports/linux/guts/fstatat.c > @@ -1,4 +1,5 @@ > /* > + * Copyright (c) 2008-2010 Wind River Systems; see > * Copyright (c) 2021 Linux Foundation; see > * guts/COPYRIGHT for information. > * > @@ -8,7 +9,13 @@ > * int rc = -1; > */ > > - rc = wrap___fxstatat(_STAT_VER, dirfd, path, buf, flags); > + struct stat64 buf64; > + /* populate buffer with complete data */ > + real_fstatat(dirfd, path, buf, flags); > + /* obtain fake data */ > + rc = wrap_fstatat64(dirfd, path, , flags); > + /* overwrite */ > + pseudo_stat32_from64(buf, ); > > /* return rc; > * } > diff --git a/ports/linux/guts/fstatat64.c > b/ports/linux/guts/fstatat64.c index c981e14..13c1143 100644 > --- a/ports/linux/guts/fstatat64.c > +++ b/ports/linux/guts/fstatat64.c > @@ -1,4 +1,5 @@ > /* > + * Copyright (c) 2008-2010 Wind River Systems; > * Copyright (c) 2021 Linux Foundation; see > * guts/COPYRIGHT for information. > * > @@ -8,7 +9,46 @@ > * int rc = -1; > */ > > - rc = wrap___fxstatat64(_STAT_VER, dirfd, path, buf, flags); > + pseudo_msg_t *msg; > + int save_errno; > + > +#ifdef PSEUDO_NO_REAL_AT_FUNCTIONS > + if (dirfd != AT_FDCWD) { > + errno = ENOSYS; > + return -1; > + } > +#endif > + if (flags & AT_SYMLINK_NOFOLLOW) { > +#ifdef PSEUDO_NO_REAL_AT_FUNCTIONS > + rc = real_lstat64(path, buf); > +#else > + rc = real_fstatat64(dirfd, path, buf, flags); > +#endif > + if (rc == -1) { > + return rc; > + } > + } else { > +#ifdef PSEUDO_NO_REAL_AT_FUNCTIONS > + rc = real_stat64(path, buf); > +#else > + rc = real_fstatat64(dirfd,
Re: [OE-core] [PATCH v4] Fixes pseudo build in loongarch64
On Tue, 30 May 2023 19:33:03 +0800 zhangjial...@loongson.cn wrote: > +#ifdef __loongarch64 No. Use the configuration mechanisms or whatever to determine the *actual distinction you care about*, which is "does glibc have __xstat64", give that a name, and use the name. Some day, someone else will make a glibc that doesn't have __xstat64. The change to adopt that should be one line in configure or the like, not hundreds of changes scattered throughout the code. > +int existed = (real_stat64(path, ) != -1); > +#else > int existed = (real___xstat64(_STAT_VER, path, ) != -1); > - > +#endif This looks suspiciously similar to the logic you introduce elsewhere for base_stat, base_fstat, etcetera. Reproducing that up here: > +#ifdef __loongarch64 > +#define base_fstatat(dirfd, path, buf, flags) real_fstatat64( dirfd, > path, buf, flags) > +#else > #define base_fstatat(dirfd, path, buf, flags) > real___fxstatat64(_STAT_VER, dirfd, path, buf, flags) > +#endif Again, shouldn't be using a specific arch flag, should be using a symbolic flag, but: Given this, maybe we should just be using base_fstatat, base_stat, etcetera, in these lines, rather than calling a specific real___foo function directly. Or alternatively, we should be using pseudo_stat, etcetera, which also exist, and I honestly don't remember why there are two sets of those. So the change should look something like - int existed = ((real___xstat64(_STAT_VER, path, ) != -1); + int existed = ((base_stat(path, ) != -1); with suitable updates to the way we define the base_foo functions, making them contingent on something like HAVE_XSTAT. This would cover the vast majority of these changes, but not all of them. > --- a/ports/linux/guts/fstat.c > +++ b/ports/linux/guts/fstat.c > @@ -7,8 +7,17 @@ > * int fstat(int fd, struct stat *buf) > * int rc = -1; > */ > - > +#ifdef __loongarch64 > + struct stat64 buf64; > +/* populate buffer with complete data */ > + real_fstat(fd, buf); > +/* obtain fake data */ > + rc = wrap_fstat64(fd, ); > +/* overwrite */ > + pseudo_stat32_from64(buf, ); > +#else > rc = wrap___fxstat(_STAT_VER, fd, buf); > +#endif > > /* return rc; > * } Here, we really do have a meaningful change; in the standard linux port, we know that we can just forward fstat to __fxstat. Probably the best model for the canonical way to fix this is to look at the oldclone/newclone subports. When you have changes this substantive, we don't want to do them with large inline #ifdefs; we want to describe these as distinct subports, which have a different set of functions. So in the hypothetical new "xstat" port, we'd have the existing fstat and __fxstat wrappers, and in the new "noxstat" port, we'd just have an fstat wrapper that uses the same logic. > diff --git a/templates/wrapfuncs.c b/templates/wrapfuncs.c > index 93bb671..14a42e2 100644 > --- a/templates/wrapfuncs.c > +++ b/templates/wrapfuncs.c > @@ -13,7 +13,9 @@ > * script if you want to modify this. */ > @body > > +#ifndef __loongarch64 > static ${type} (*real_${name})(${decl_args}) = ${real_init}; > +#endif > > ${maybe_skip} I don't understand how this part can work at all; I was under the impression that we actually did in fact use these real_foo objects, and this seems to remove them all, so I don't even get how this is compiling or running, because after this, there's no real_foo function pointers at all? Unfortunately, it turns out bitrot has taken my laptop and I can't build pseudo on it natively because _STAT_VER no longer exists. So I think the hypothetical distant future in which we conceivably care about a system on which the real___xstatat(_STAT_VER, ...) calls fail and isn't __longarch64 actually happened over a year ago and we just didn't notice. "Oops." -s -=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#181938): https://lists.openembedded.org/g/openembedded-core/message/181938 Mute This Topic: https://lists.openembedded.org/mt/99217464/21656 Group Owner: openembedded-core+ow...@lists.openembedded.org Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [OE-core] pseudo database integrity checking
On Mon, 27 Jun 2022 14:52:39 +0100 Richard Purdie wrote: > b) We could add a new command to run an integrity check on the DB to > pseudo. If we do that, we would then be able to show the user a decent > error and above the timeout issue. The question is where/when to > trigger it and whether races could occur against the check (e.g. where > multiple fakeroot tasks are running in parallel against the same > WORKDIR). I thought there was a function that did roughly this once at some point. -s -=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#167323): https://lists.openembedded.org/g/openembedded-core/message/167323 Mute This Topic: https://lists.openembedded.org/mt/92020938/21656 Group Owner: openembedded-core+ow...@lists.openembedded.org Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [OE-core] [pseudo][PATCH] do not abort on hard link path mismatch
On Mon, 25 Apr 2022 14:01:58 -0400 "C. Andy Martin" wrote: > I agree this is not the ideal solution. However, I was basing this > compromise on this commit: Yeah. Come to think of it, directory mismatches are already probably blowing up differently because we're more concerned about that. Probably okay. -s -=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#164840): https://lists.openembedded.org/g/openembedded-core/message/164840 Mute This Topic: https://lists.openembedded.org/mt/90686441/21656 Group Owner: openembedded-core+ow...@lists.openembedded.org Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [OE-core] [pseudo][PATCH] do not abort on hard link path mismatch
On Mon, 25 Apr 2022 10:49:24 -0400 "C. Andy Martin" wrote: > - msg->result = RESULT_ABORT; > - goto op_exit; > + if (msg->nlink == 1) { > + msg->result = > RESULT_ABORT; > + goto op_exit; > + } I'm distrustful of this because I feel like, in the case where we have apparently-stale information in the database, we really should be coming back with "we don't have valid database information", not yielding what's probably incorrect/old data. I'm also concerned a bit because nlink is never 1 for a directory... -s -=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#164832): https://lists.openembedded.org/g/openembedded-core/message/164832 Mute This Topic: https://lists.openembedded.org/mt/90686441/21656 Group Owner: openembedded-core+ow...@lists.openembedded.org Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [OE-core] [PATCH 2/6] test-openat: Consider device as well as inode number
On Thu, 9 Sep 2021 16:36:10 +0100 Richard Purdie wrote: > It just so happens that my /home/mac and /home directories have the > same inode number but on different filesystems. Nice catch! -s -=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#155864): https://lists.openembedded.org/g/openembedded-core/message/155864 Mute This Topic: https://lists.openembedded.org/mt/85487315/21656 Group Owner: openembedded-core+ow...@lists.openembedded.org Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [OE-core] [PATCH 3/6] pseudo_client: Do not pass null argument to pseudo_diag()
On Thu, 9 Sep 2021 16:36:11 +0100 Richard Purdie wrote: > pseudo_client.c:848:17: warning: ‘%s’ directive argument is null > [-Wformat-overflow=] 848 | pseudo_diag("couldn't > allocate absolute path for '%s'.\n", I'm sort of curious as to whether pseudo_root_path is ever actually called on a null path, but eh, checks are cheap. -s -=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#155863): https://lists.openembedded.org/g/openembedded-core/message/155863 Mute This Topic: https://lists.openembedded.org/mt/85487316/21656 Group Owner: openembedded-core+ow...@lists.openembedded.org Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [OE-core] [PATCH 5/6] pseudo_client: Make msg static in pseudo_op_client
On Thu, 9 Sep 2021 16:36:13 +0100 Richard Purdie wrote: > - pseudo_msg_t msg = { .type = PSEUDO_MSG_OP }; > + static pseudo_msg_t msg; > + msg = (pseudo_msg_t) { .type = PSEUDO_MSG_OP }; Looks good to me. I'm honestly sort of stunned that the return of the local variable's address caused a problem, even though it's theoretically undefined, because it's only ever checked against is-null, but I'm also sort of stunned that I got away with it for that long. -s -=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#155862): https://lists.openembedded.org/g/openembedded-core/message/155862 Mute This Topic: https://lists.openembedded.org/mt/85487319/21656 Group Owner: openembedded-core+ow...@lists.openembedded.org Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [OE-core] pseudo: Outdated records for newly-ignored paths in database cause mismatches
On Mon, 9 Aug 2021 13:19:51 +0100 "Mike Crowe via lists.openembedded.org" wrote: > Cleaning the work directory makes the problem go away because that > deletes the pseudo databases. > > Does the above make sense as an explanation for these errors? If so, > is there a good way to avoid these errors? Good diagnostic work, makes sense to me. It would make some sense for pseudo to ignore mismatches involving ignored paths, but it wasn't originally designed with the ignored paths concept, so it currently doesn't. -s -=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#154620): https://lists.openembedded.org/g/openembedded-core/message/154620 Mute This Topic: https://lists.openembedded.org/mt/84766871/21656 Group Owner: openembedded-core+ow...@lists.openembedded.org Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [OE-core] [PATCH pseudo 4/4] Do not return address of local variable - unverified
On Thu, 29 Jul 2021 14:37:28 +0200 "Damian Wrobel" wrote: > the PSEUDO_MSG_OP is being unconditionally assigned to the msg.type > before any real usage of the 'msg' structure. So, if I'm not mistaken > that code was already tested and didn't work well and was reverted > here[1]. I don't think that's identical. msg = (pseudo_msg_t) { .type = PSEUDO_MSG_OP }; zeros out a number of other fields. Anyway, if it fails with msg declared static, I'd be interested in a minimal reproducer for it, I don't see anything obvious that ought to care about it, but I would easily believe that the other fields not being zeroed out would cause weirdness, which is why I suggested a complete assignment to it, not just overwriting the .type field. -s -=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#154250): https://lists.openembedded.org/g/openembedded-core/message/154250 Mute This Topic: https://lists.openembedded.org/mt/84526799/21656 Group Owner: openembedded-core+ow...@lists.openembedded.org Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [OE-core] [PATCH pseudo 4/4] Do not return address of local variable
On Wed, 28 Jul 2021 13:49:17 -0700 Andre McCurdy wrote: > If the caller only cares about yes/no then how about returning 1/0 > instead of a pointer? Other callers are actually using the response, because the pseudo_msg_t* returned from this is how, say, OP_STAT responds with the stat information it got from the server. It's just chroot, specifically, that wants to know that the client accepted-and-processed the chroot, but there's no meaningful additional information past that. I suppose in theory pseudo_client_op could be split into one function for ops with message returns and another for ops with only success/failure, or "just ignored", but it doesn't seem rewarding. In Go, I'd probably have done it as func (c *PseudoClient) Op(whatever goes here) (*Message, error) and this would just be returning (nil, nil) or (nil, some-meaningful-error) in the chroot case, but here we are in C with single return values. :) -s -=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#154219): https://lists.openembedded.org/g/openembedded-core/message/154219 Mute This Topic: https://lists.openembedded.org/mt/84479678/21656 Group Owner: openembedded-core+ow...@lists.openembedded.org Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [OE-core] [PATCH pseudo 4/4] Do not return address of local variable
On Wed, 28 Jul 2021 11:36:22 +0200 "Damian Wrobel" wrote: > Do I correctly assume that pseudo_client_op() has to be fully > reentrant? No. It's never been even a tiny bit reentrant. We used to do the allocate and free thing, and it was incredibly expensive, and the nature of the thing requires confidence that we never, ever, have more than one thing writing and reading over the socket at a time, so it's just Not Reentrant. During one call to pseudo_client_op, there will never be another, and all the IPC stuff uses a single consistent local buffer that it returns the address of. Declaring that as static without changing the initializer would indeed break everything -- we rely on the initializer working. Changing it to static means it only gets initialized once... Changing it to: static pseudo_msg_t msg; msg = pseudo_msg_t { .type = PSEUDO_MSG_OP }; would probably be fine, because then it'd be initialized. Otherwise, we'd get failures when msg got overwritten and reused. Or just changing `result = ` to something like `result = _data`, which would be nonsensical but it turns out not to matter, as the only caller that reaches this case is the caller that's just checking yes/no "is the return value not a null pointer". -s -=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#154214): https://lists.openembedded.org/g/openembedded-core/message/154214 Mute This Topic: https://lists.openembedded.org/mt/84479678/21656 Group Owner: openembedded-core+ow...@lists.openembedded.org Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [OE-core] [PATCH pseudo 4/4] Do not return address of local variable
On Tue, 27 Jul 2021 18:30:33 +0200 Damian Wrobel wrote: > The returned pointer has to be freed by the caller not by the callee > function itself. So, this predates the public release, but long ago, that was indeed how it worked, and then LONG ago it was changed so that the pseudo_ipc stuff always used the same object for its returns, so we weren't doing alloc/free cycles all the time. Which means that, in every *other* code path, if we return a non-nil msg, it *must not* be freed. I think probably the solution is to change that object to be static. We can't make callers free the results unless we want them ALL to be freed, which we absolutely don't, that's devastatingly expensive. There is exactly one call with OP_CHROOT, and all it checks is whether the return is null or not-null. I'd be mildly surprised by the theoretically-invalid address of stack garbage actually causing a problem on most modern systems, except that I think some systems have started doing stack guards. But all we care about here is that the address returned be a valid non-null pointer. Heck, we could use _data, that already exists, is already static, and we don't care about it. (The reason the `msg` in that function isn't static is so it gets its initializer every time. This is not a great reason.) -s -=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#154176): https://lists.openembedded.org/g/openembedded-core/message/154176 Mute This Topic: https://lists.openembedded.org/mt/84479678/21656 Group Owner: openembedded-core+ow...@lists.openembedded.org Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [OE-core] [PATCH pseudo 1/4] Remove -fno-strict-aliasing and -Wno-deprecated-declarations
On Tue, 27 Jul 2021 18:35:50 +0200 "Damian Wrobel" wrote: > Based on my experience, It's usually better to see this warning and > fix the code instead of relying on the back magic. Yes, but in the case of the strict aliasing stuff and function pointer stuff, we really, really, *can't* not rely on the magic, because pseudo *is* the magic. -s -=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#154175): https://lists.openembedded.org/g/openembedded-core/message/154175 Mute This Topic: https://lists.openembedded.org/mt/84479676/21656 Group Owner: openembedded-core+ow...@lists.openembedded.org Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [OE-core] [PATCH pseudo 1/4] Remove -fno-strict-aliasing and -Wno-deprecated-declarations
On Tue, 27 Jul 2021 13:49:03 +0200 "Damian Wrobel" wrote: > -# no-strict-aliasing is needed for the function pointer trickery. > pseudo_wrappers.o: $(GUTS) pseudo_wrappers.c pseudo_wrapfuncs.c > pseudo_wrapfuncs.h pseudo_tables.h > - $(CC) -fno-strict-aliasing $(CFLAGS) $(CFLAGS_PSEUDO) > -D_GNU_SOURCE -c -o pseudo_wrappers.o pseudo_wrappers.c I would be really uncomfortable relying on that not being needed, even if it happened to work in specific cases. The function pointer trickery is black magic and very much undefined behavior, and warning the compiler away from it seems reasonably likely to be advantageous. -s -=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#154170): https://lists.openembedded.org/g/openembedded-core/message/154170 Mute This Topic: https://lists.openembedded.org/mt/84479676/21656 Group Owner: openembedded-core+ow...@lists.openembedded.org Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [OE-core] [PATCH pseudo 2/4] Do not pass null argument to pseudo_diag()
On Tue, 27 Jul 2021 13:49:04 +0200 "Damian Wrobel" wrote: > pseudo_diag("couldn't allocate absolute path for > '%s'.\n", > - path); > + path ? path : "null"); Is there any actual code path where pseudo_root_path gets called on a null path? I don't really object to the change, but I think that's caught by guards elsewhere. -s -=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#154169): https://lists.openembedded.org/g/openembedded-core/message/154169 Mute This Topic: https://lists.openembedded.org/mt/84479888/21656 Group Owner: openembedded-core+ow...@lists.openembedded.org Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [OE-core] [PATCH pseudo 4/4] Do not return address of local variable
On Tue, 27 Jul 2021 13:49:06 +0200 "Damian Wrobel" wrote: > Fixes the following warning: > pseudo_client.c: In function ‘pseudo_client_op’: > cc1: warning: function may return address of local variable > [-Wreturn-local-addr] pseudo_client.c:1592:22: note: declared here >1592 | pseudo_msg_t msg = { .type = PSEUDO_MSG_OP }; > | ^~~ > > Signed-off-by: Damian Wrobel > --- > pseudo_client.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/pseudo_client.c b/pseudo_client.c > index 2583bca..f1d09ff 100644 > --- a/pseudo_client.c > +++ b/pseudo_client.c > @@ -1889,7 +1889,7 @@ pseudo_client_op(pseudo_op_t op, int access, > int fd, int dirfd, const char *path case OP_CHROOT: > if (pseudo_client_chroot(path) == 0) { > /* return a non-zero value to show > non-failure */ > - result = > + result = pseudo_msg_dup(); This is a memory leak. That said, I have no idea how the underlying bug escaped notice all this time, it's definitely a bug. I think it is actually safe to just make msg be static, because pseudo_client_op is protected by a lock and is never executed more than once at a time. On reflection: I think the way it worked is that in that case, the actual message isn't looked at, just checked for nullness, but this is still undefined behavior because the result is a pointer to storage after the storage's lifetime, and formally you can't even check those for "is or isn't null". -s -=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#154168): https://lists.openembedded.org/g/openembedded-core/message/154168 Mute This Topic: https://lists.openembedded.org/mt/84479678/21656 Group Owner: openembedded-core+ow...@lists.openembedded.org Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [OE-core] [PATCH][pseudo 4/7] Fix some memory leaks
For reference, I believe the rationale on these originally was "none of these functions happen repeatedly, in general, and the extra complexity and potential to get the logic wrong isn't appealing", but I don't think I object to these. -s -=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#146549): https://lists.openembedded.org/g/openembedded-core/message/146549 Mute This Topic: https://lists.openembedded.org/mt/79523829/21656 Group Owner: openembedded-core+ow...@lists.openembedded.org Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [OE-core] [PATCH 2/4] pseudo: Ignore mismatched inodes from the db
On Mon, 28 Sep 2020 22:42:18 +0100 "Richard Purdie" wrote: > It helps to know that! I wasn't sure if you'd hate the path filtering. It was on my to-do list at one point to be able to denote "the pseudo filesystem" in some way and just politely ignore everything outside it. And, possibly, switch to keeping paths relative to it in some way. But it looked very expensive, and honestly one of the things it was waiting on was my vague belief that what pseudo really needs is to switch to using length-prefixed strings so it can stop constantly calling strlen. -s -=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#142907): https://lists.openembedded.org/g/openembedded-core/message/142907 Mute This Topic: https://lists.openembedded.org/mt/77174127/21656 Group Owner: openembedded-core+ow...@lists.openembedded.org Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [OE-core] [PATCH 2/4] pseudo: Ignore mismatched inodes from the db
On Mon, 28 Sep 2020 15:51:53 +0100 "Richard Purdie" wrote: > I understand. I have strong evidence that the current handling of such > a case does the wrong thing though as copying the data from the > original inode leads to pretty bad corruption in its own right. Yes. But if you had to choose between (1) discard the possibly-bad data, and (2) abort(), 2 would be a MUCH better fix. Don't treat this as a thing to be worked around. Treat it as a giant red flag that *we no longer have a sound reason to think that the database is valid*. > In many ways I'd like to make these corner cases hard errors. In order > to do that we need to ensure we're not hitting them though and to do > that we need the next patch. Yeah. > Once we have the ability to ignore subtrees, we could just hard error > for the potential corruption cases and force those issues to be > addressed properly. I think that is the right path. -s -=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#142860): https://lists.openembedded.org/g/openembedded-core/message/142860 Mute This Topic: https://lists.openembedded.org/mt/77174127/21656 Group Owner: openembedded-core+ow...@lists.openembedded.org Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [OE-core] [PATCH 2/4] pseudo: Ignore mismatched inodes from the db
On Mon, 28 Sep 2020 14:38:01 +0100 "Richard Purdie" wrote: > This can happen when files are deleted outside of pseudo context and > the inode is reused by a new file which pseduo then "sees". I'm just going to say again: The **ENTIRE REASON** pseudo exists to replace fakeroot is that fakeroot did this, and it consistently, repeatedly, reliably, resulted in horrible database corruption. If files that pseudo knows about are being deleted outside the pseudo context, that is violating a crucial precondition, and **you cannot then trust the database to make sense**. Yes, you can throw away some of the specific files when you detect them -- pseudo already did this for specific cases, like directory vs plain file mismatches -- but you should always view it as a serious bug in the rest of the environment. If it's **really** necessary to allow things to corrupt the filesystem, we need a way to tell pseudo of **specific** files that it should be forgetting about or disregarding. But, in general, "a thing that isn't under pseudo is modifying the part of the filesystem pseudo thinks it owns" means "your entire database is presumptively corrupt and you should not be guessing at which parts of it might have survived that". I know it seems like this could conceivably be correct, but our experience with attempts to fix it up in other ways was that it always ended up resulting in very weird and incomprehensible corruption. -s -=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#142853): https://lists.openembedded.org/g/openembedded-core/message/142853 Mute This Topic: https://lists.openembedded.org/mt/77174127/21656 Group Owner: openembedded-core+ow...@lists.openembedded.org Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [OE-core] [PATCH] pseudo: Remove mismatched inodes from the db
On Tue, 22 Sep 2020 18:22:18 +0100 "Richard Purdie" wrote: > The "fun" here is that "make install" is rewriting Makefiles within > the source code tree. The install step runs under pseudo, earlier > ones do not. > If we rerun earlier non-pseudo steps, the Makefiles get reset to their > original state, then "make install" modifies them again. ugh. Okay, so. A vague thought about how to perhaps approach this: A post-install step, which basically does: pseudo cp Makefile Makefile.pseudo pseudo rm Makefile [not-pseudo] cp Makefile.pseudo Makefile pseudo rm Makefile.pseudo So that, at the end of the post-install, the files pseudo doesn't want to know about are forgotten. Possibly there ought to be a "forget about this directory" option/command. -s -=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#142727): https://lists.openembedded.org/g/openembedded-core/message/142727 Mute This Topic: https://lists.openembedded.org/mt/77012533/21656 Group Owner: openembedded-core+ow...@lists.openembedded.org Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [OE-core] [PATCH] pseudo: Remove mismatched inodes from the db
On Tue, 22 Sep 2020 14:18:24 +0100 "Richard Purdie" wrote: > This can happen when files are deleted outside of pseudo context and > the inode is reused by a new file which pseduo then "sees". In terms of the original design: This would be considered a usage bug, and pseudo issues diagnostics for that. Once files are owned by an instance of pseudo, it expects to "see" every change to those files, and if you don't do that, then yes, the database is corrupt. But in some cases, the behavior was something like "a move within a pseudo filesystem" or something similar, and losing the data would also be bad. Basically, the original design philosophy is that you should *never* be modifying things pseudo owns outside of pseudo. And that does impose a performance cost on things, *but*, it also gives us a lot of confidence in results. So my preference on "deleting files outside of pseudo context, inode gets reused" would be "don't do that then"; this is why pseudo reports diagnostics for those. Being able to tell you what the old path was in such cases was actually one of the primary reasons pseudo exists. -s -=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#142725): https://lists.openembedded.org/g/openembedded-core/message/142725 Mute This Topic: https://lists.openembedded.org/mt/77012533/21656 Group Owner: openembedded-core+ow...@lists.openembedded.org Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [OE-core] [pseudo][PATCH 1/2] xattr: Fixed corrupting UID when running setfacl -m on a directory
On Fri, 26 Jun 2020 18:33:39 +0100 "Richard Purdie" wrote: > There is one patch I'm not going to put there as looking at it, I > suspect it needs reworking and really isn't upstreamable, I think the > rest are appropriate given OE is one of pseudo's main users and can > hence influence its development. > > Hope that is ok with you seebs! Seems reasonable to me. SOME DAY I will have free time. :P -s -=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#140031): https://lists.openembedded.org/g/openembedded-core/message/140031 Mute This Topic: https://lists.openembedded.org/mt/75120999/21656 Group Owner: openembedded-core+ow...@lists.openembedded.org Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [OE-core] [pseudo][PATCH 1/2] xattr: Fixed corrupting UID when running setfacl -m on a directory
On Fri, 26 Jun 2020 16:56:48 +0200 "Johannes Beisswenger" wrote: > The OP_CREATE/REPLACE/SET_XATTR operations in pseudo_op() don't use > msg_header.mode at all, it is only used for DB sanity checks, which > in turn only use the non-permission bits. Right, the relevant part is the immediately following wrap_chmod/wrap_fchmod. I had forgotten about this chunk of code. And... that leads us to the realization of how it is possible that this ever didn't bite us before. The historical reason this exists is that at some point, GNU tar decided that it was a crime against nature, humanity, and God to use a stable existing API when a new API existed which could do the same thing at hundreds of times the computational cost, and abandoned all use of chmod() in circumstances when extended attributes are available. But in practice, this only happens for regular filesystem modes that do not require an ACL, and allow us to replace "here's your 12 bits of values" with "here's a small binary blob you can decode expensively". But usually that's all that's present, so we just hit the path where we call chmod/fchmod and return early. For us to hit this, we have to have gotten a system.posix_acl_access message which contains access list values other than those, which of course never happens, because who would actually use ACLs for their intended purpose, instead of as a ridiculously expensive way to implement chmod? And yeah, I think you're right -- this assignment is just plain unneeded. (I think the Darwin port is long-dead because Apple's security stuff makes it a pain to work with. Basically, "can we get this working on Darwin" was a high priority and common request until about three days after I got it running, and I don't think anyone's asked about it since.) -s -=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#140019): https://lists.openembedded.org/g/openembedded-core/message/140019 Mute This Topic: https://lists.openembedded.org/mt/75120999/21656 Group Owner: openembedded-core+ow...@lists.openembedded.org Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [OE-core] [pseudo][PATCH 1/2] xattr: Fixed corrupting UID when running setfacl -m on a directory
On Fri, 26 Jun 2020 11:40:31 +0200 "Johannes Beisswenger" wrote: > The file mode was accidentally overwritten with only the permission > bits, causing the server to falsely assume that the database was > corrupted (because the msg_header.mode did not contain S_IFDIR > anymore) even though it was the client doing the corruption. > In practice that had the effect of leaking the UID of the user, into > the pseudo environment. Good catch, but we should still be masking in the permissions bits, we just shouldn't be overwriting the non-permissions bits, I think? -s -=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#140015): https://lists.openembedded.org/g/openembedded-core/message/140015 Mute This Topic: https://lists.openembedded.org/mt/75120999/21656 Group Owner: openembedded-core+ow...@lists.openembedded.org Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [OE-core] [pseudo][PATCH] pseudo_ipc.h: Fix enum typedef
On Fri, 8 May 2020 08:09:58 +0200 Jacob Kroon wrote: > Hi Peter, > > On 5/3/20 6:26 PM, Seebs wrote: > > On Sun, 3 May 2020 17:49:09 +0200 > > "Jacob Kroon" wrote: > > > >> The type has never been used, so an alternative fix would be to get > >> rid of it altogether, and just keep the enum identifiers. > > > > I think I anticipated the type being used for something but it did > > indeed not happen. It could also just be changed to 'enum [type] > > { ... }', I guess. > > > > But I think your fix is the simplest and closest to what I probably > > thought I was doing at the time. > > > > Do you have any plans on merging this, and also the pending patches > in oe-core, anytime soon ? Then we could bump the pseudo git rev in > oe-core and remove those patches, if not then I can add the patch to > the pseudo recipe. Any way you'd prefer ? It probably makes sense to add things to the pseudo recipe for now, I'm still trying to catch up on my task backlog from March. -s -=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#138057): https://lists.openembedded.org/g/openembedded-core/message/138057 Mute This Topic: https://lists.openembedded.org/mt/73948932/21656 Group Owner: openembedded-core+ow...@lists.openembedded.org Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [OE-core] [pseudo][PATCH] pseudo_ipc.h: Fix enum typedef
On Sun, 3 May 2020 17:49:09 +0200 "Jacob Kroon" wrote: > The type has never been used, so an alternative fix would be to get > rid of it altogether, and just keep the enum identifiers. I think I anticipated the type being used for something but it did indeed not happen. It could also just be changed to 'enum [type] { ... }', I guess. But I think your fix is the simplest and closest to what I probably thought I was doing at the time. -s -=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#137780): https://lists.openembedded.org/g/openembedded-core/message/137780 Mute This Topic: https://lists.openembedded.org/mt/73948932/21656 Group Owner: openembedded-core+ow...@lists.openembedded.org Unsubscribe: https://lists.openembedded.org/g/openembedded-core/leave/8023207/1426099254/xyzzy [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [OE-core] [pseudo][PATCH] pseudo_ipc.h: Fix enum typedef
On Sun, 3 May 2020 06:27:12 +0200 "Jacob Kroon" wrote: > 'pseudo_access_t' is a type, so use typedef. > > Fixes building pseudo with gcc 10 where -fno-common is the default. Wow! That's amazing, and yes, that's a bug, and the fix looks right to me. I don't know how that got missed all these years. (But don't rely on this evaluation too heavily just yet, I haven't actually looked at the code more carefully, but this seems obvious.) -s -=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#137761): https://lists.openembedded.org/g/openembedded-core/message/137761 Mute This Topic: https://lists.openembedded.org/mt/73948932/21656 Group Owner: openembedded-core+ow...@lists.openembedded.org Unsubscribe: https://lists.openembedded.org/g/openembedded-core/leave/8023207/1426099254/xyzzy [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [OE-core] [PATCH] pseudo: add macro guard for seccomp
On Mon, 27 Apr 2020 14:04:16 +0800 "kai" wrote: > | ports/linux/pseudo_wrappers.c:129:14: error: > ‘SECCOMP_SET_MODE_FILTER’ undeclared (first use in this function) > |if (cmd == SECCOMP_SET_MODE_FILTER) { Should the test be against this being #defined, rather than the separate SYS__seccomp? -s -=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#137524): https://lists.openembedded.org/g/openembedded-core/message/137524 Mute This Topic: https://lists.openembedded.org/mt/73298568/21656 Group Owner: openembedded-core+ow...@lists.openembedded.org Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [OE-core] [pseudo][PATCH] pseudo_util.c: Open file with O_CLOEXEC to avoid fd leak
On Thu, 9 Apr 2020 06:30:37 + "Yu, Mingli" wrote: > We are trying to fix the issue as > http://bugzilla.yoctoproject.org/show_bug.cgi?id=13311 stated. Hmm. I think there's a more subtle logic bug here, and also one of the diagnostics there is a debug message I didn't notice when I made the commit, but it never comes up unless you run with PSEUDO_DEBUG_FILE. I'm gonna think about it more. I'm now about half convinced that the patch is actually fine as-is for clients, it's just on the server side that I think we'd see problems. -s -=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#137155): https://lists.openembedded.org/g/openembedded-core/message/137155 Mute This Topic: https://lists.openembedded.org/mt/72889243/21656 Group Owner: openembedded-core+ow...@lists.openembedded.org Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [OE-core] [pseudo][PATCH] pseudo_util.c: Open file with O_CLOEXEC to avoid fd leak
On Thu, 9 Apr 2020 10:38:41 +0800 "Yu, Mingli" wrote: > - fd = open(pseudo_path, O_WRONLY | O_APPEND | O_CREAT, 0644); > + fd = open(pseudo_path, O_WRONLY | O_APPEND | O_CREAT | > O_CLOEXEC, 0644); I'm not confident in this. This code is shared between the pseudo client and the pseudo server. In the pseudo server, it's possible that we coincidentally end up with fd being 2 already. In that case, we'd end up with this fd on 2, but then on exec (such as when execing the server) we'd possibly end up with it closed? Looking more closely, dup2 will clear the CLOEXEC flag, so in the server path, if prefer_fd is 2 and fd doesn't start out as 2, we end up with it cleared... But also so far as I can tell the code around the dup2 call there is probably generally wrong; it's explicitly closing the previous holder of that descriptor, which is unnecessary, and checking whether the file descriptor is already the desired one, which is also unnecessary. So this might need to be revisited. Looking at it more, the only time this seems like it should be relevant would be in the case where we're in the pseudo client, but PSEUDO_DEBUG_FILE is set in the environment. Otherwise, the client should end up just using stderr, and the server shouldn't care because it's expecting this to be fd 2 and we probably want the child process to end up with fd 2 open. What's the observed failure mode that we're fixing? -s -=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#137132): https://lists.openembedded.org/g/openembedded-core/message/137132 Mute This Topic: https://lists.openembedded.org/mt/72889243/21656 Group Owner: openembedded-core+ow...@lists.openembedded.org Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [OE-core] [PATCH] pseudo: Force seccomp to return success when in fact doing nothing
On Sat, 4 Apr 2020 14:32:03 -0700 "Andre McCurdy" wrote: > Also, since prctl() is Linux specific, it looks like this patch will > make pseudo Linux specific. Is that OK? If so maybe worth making an > official statement that OE is only supported for Linux hosts? We have existing hooks for making things like this be in a Linux-specific port directory. I don't think the Darwin port still works; people asked me about it a lot, so I made it work and never heard about it again. :) -s -=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#137024): https://lists.openembedded.org/g/openembedded-core/message/137024 Mute This Topic: https://lists.openembedded.org/mt/72759808/21656 Group Owner: openembedded-core+ow...@lists.openembedded.org Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [OE-core] [PATCH 13/25] pseudo: adjust for attr 2.4.48
On Wed, 18 Dec 2019 17:24:21 +0100 Alexander Kanavin wrote: > You can also supply your own definition of ENOATTR, as it doesn't > change. That way lies EVEN MORE madness. I don't have any guarantee that it won't be changed at some point, and... I mean, silly though it sounds to talk about "defined" or "undefined" when working on pseudo, it's *really* not okay for non-implementation code to be defining a symbol starting with a capital E like that. I am not sure why the attr people hate compatibility with existing code, but probably because they're monsters who are motivated only by a desire to crush the hopes and dreams of others. That's usually why people make backwards-incompatible header changes. -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] [PATCH 13/25] pseudo: adjust for attr 2.4.48
On Wed, 18 Dec 2019 17:15:34 +0100 Alexander Kanavin wrote: > On Wed, 18 Dec 2019 at 16:27, Seebs wrote: > > > On Wed, 18 Dec 2019 15:37:46 +0100 > > Alexander Kanavin wrote: > > > > > +Latest versions of attr have removed the xattr.h header, > > > +with the rationale that libc is providing the same wrappers. > > > > I'm a bit concerned about this -- will this still work with *older* > > versions of xattr? > > > > It won't, as attr upstream moved the definition of ENOATTR from one > header file to another. > > attr is supplied by oe-core though, so we have control over which > version is used. So basically, if I want to maintain support in pseudo for older xattr, I need to figure out a way to try both. Thanks, attr devs. :P (Pseudo is still sometimes used outside of oe-core.) -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] [PATCH 13/25] pseudo: adjust for attr 2.4.48
On Wed, 18 Dec 2019 15:37:46 +0100 Alexander Kanavin wrote: > +Latest versions of attr have removed the xattr.h header, > +with the rationale that libc is providing the same wrappers. I'm a bit concerned about this -- will this still work with *older* versions of xattr? -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] [PATCH] pseudo: Drop static linking to sqlite3
On Mon, 11 Nov 2019 08:17:30 -0600 Mark Hatle wrote: > First (early) version of pseudo would fork for the server, there > wasn't a seperate executed server and we had all sorts of problems > with dynamic libraries being loaded and loaded from the correct paths > (due to the LD_PRELOAD). > > Moving to two process spaces, LD_PRELOAD and the pseudo executable > happened pretty early in development, but I suspect some of this is > left over from that. To be picky: pseudo *still does this*. If it can't find the server, it clears the environment and respawns it. There were problems with this for the longest time, which turned out to be because I was calling setenv to clean the environment, and bash *overrides setenv*, so if the program spawning the pseudo server was bash, things went wrong. It should actually be fixed now and work pretty well. -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] [PATCH] pseudo: Drop static linking to sqlite3
On Sat, 09 Nov 2019 16:30:41 + Richard Purdie wrote: > I did talk briefly to Mark (also cc'd) as he wrote the original patch > and he thought it was possibly because the client was also linking > against sqlite3 and due to the other things the client does, that was > problematic. It *shouldn't* link against sqlite3. But! The commit in question refers to RHEL5 and LD_LIBRARY_PATH, and I think that shook loose a memory: I think at one point, we had a Crucial Bug Fix in sqlite3, in our build system, and if we didn't statically link, there was a risk of getting the broken version at runtime. > The client lib doesn't and the server side should behave just like any > other linux binary afaik so we should be ok with a dynamicly linked > sqlite3? Yes. The issue here was, I believe, not "dynamically-linked sqlite3 per se", but "dynamic linking, plus LD_LIBRARY_PATH, picking an sqlite3 which caused us specific problems". In the Yocto environment, I think we're reasonably sure that we always get a clean Yocto-built sqlite3, and that *should* be fine. So I'd say go for it, but if you see weird sqlite3 stuff that happens only very occasionally, look at this first. :P -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] [PATCH] pseudo: Drop static linking to sqlite3
On Sat, 9 Nov 2019 15:35:59 + Richard Purdie wrote: > Back in 2010[1] we made pseudo statically link against sqlite3. Since > then the world has changed, pseudo now has separate processes for the > database in the server and the client and they have separate linking > commands. I'm not sure what that has to do with the reasons for it being static? Not that I remember exactly anymore, but I think we were hitting problems caused if pseudo was dynamically linked to sqlite, but I don't remember what they were. I'm not entirely sure I know what you mean by "separate processes for the database in the server and the client". I don't think libpseudo.so should ever do any database things at all, and it shouldn't need to link to sqlite. > which occurs if sqlite3-native was built on a machine with glibc 2.28 > or later and pseudo-native is being built on glibc before that. I don't think pseudo's ever been *expected* to work when built linked against things built against different libc. It is *way* too full of unholy magics that depend on knowing which libc it's using. That said, the pseudo server also shouldn't really need to be statically-linked, the only times it ever runs under pseudo, it just immediately tries to escape and not run that way. > There appears to be no easy way to avoid this other than adding a > copy of sqlite3 into the pseudo recipe. Given the static linking > doesn't seem to be required any longer due to the separate processes, > drop that to fix those issues. I don't understand the "separate processes" thing. So far as I know, pseudo has never talked to sqlite from the client. I think Jason Wessel had experimental patches at one point to do client-local sqlite work to avoid the IPC overhead, but I don't think it ever got merged because it turns out the IPC overhead is insignificant compared to everything else. -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] [PATCH] pseudo: Add statx support to fix fedora30 issues
On Thu, 7 Nov 2019 20:04:44 + Richard Purdie wrote: > Modern distros (e.g. fedora30) are starting to use the new statx() > syscall through the newly exposed glibc wrapper function in software > like coreutils (e.g. the ls command). Add support to intercept this > to pseudo. I think this should be okay. If there's no real statx() wrapper, the real_statx() call probably fails, but also things probably won't have thought it existed/worked. Worth double-checking that things still run with this in place on a system that doesn't have a statx wrapper, when configuring and building things like coreutils that will check for it. -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] [PATCH v2] pseudo: Upgrade to latest to fix openat() with a directory symlink [NAK]
On Sat, 3 Aug 2019 05:33:46 -0700 Khem Raj wrote: > Will this fix the file ownership issue that we see with Glibc-locale > packages from time to time? I have no idea. Since I haven't got a reliable reproducer for it, I can't test it in a sane way. -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] [PATCH v2] pseudo: Upgrade to latest to fix openat() with a directory symlink [NAK]
On Fri, 2 Aug 2019 12:07:33 -0500 Seebs wrote: > Note that there's no lstat, and no AT_SYMLINK_NOFOLLOW. Which is to > say, these stats will be following the symlink even though O_NOFOLLOW > was set. I can probably patch this in a bit. Followup: Patch applied to master, but also in addition to fixing the stat calls, I had to use `flags_NOFOLLOW` rather than `flags|O_NOFOLLOW`. I am sort of amazed at how much DIDN'T break right away with that one. -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] [PATCH v2] pseudo: Upgrade to latest to fix openat() with a directory symlink [NAK]
On Fri, 2 Aug 2019 11:27:45 -0500 Jason Wessel wrote: > The sequence of openat() followed by an fstat() on the opened file > handle, will erase the pseudo uid entry for the symlink, as shown by > the following lstat() in test 5. The culprit appears to be the > fstat(), but it could be something much more complex than that... > The next step is to figure out why the recent change to openat() to > address test case 1, caused this new problem. I suspect I know that one, although I'm not sure I know the details. Pseudo will destroy entries of incompatible directory-entry types; for instance, if it has the same path listed as both a plain file and a directory. But consider, from openat.c: #ifdef PSEUDO_NO_REAL_AT_FUNCTIONS rc = real___xstat64(_STAT_VER, path, ); #else rc = real___fxstatat64(_STAT_VER, dirfd, path, , 0); #endif Note that there's no lstat, and no AT_SYMLINK_NOFOLLOW. Which is to say, these stats will be following the symlink even though O_NOFOLLOW was set. I can probably patch this in a bit. -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] [PATCH v2] pseudo: Upgrade to latest to fix openat() with a directory symlink [NAK]
On Thu, 1 Aug 2019 16:37:26 -0500 Jason Wessel wrote: > It seems to have caused really odd problems with the oe link > management that were not there previously, such as: > > > WARNING: pinentry-1.1.0-r0 do_package_qa: QA Issue: > pinentry: /usr/bin/pinentry is owned by uid 5002, which is the same > as the user running bitbake. This may be due to host contamination > [host-user-contaminated] > > I'll continue to look into the problem. There's a possibility that the right flag is something like (flags_NOFOLLOW)&&!(flags_PATH) or something like that. There's a handful of references to this in wrapfuncs.in in ports/unix and ports/linux. -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] [PATCH] pseudo: Upgrade to latest to fix openat() with a directory symlink
On Thu, 1 Aug 2019 10:55:45 -0700 Jason Wessel wrote: > Many thanks to Peter Seebach for fixing the problem in the pseudo code > to use the same logic which was already there for the > AT_SYMLINK_NOFOLLOW. Special credit goes to Past Seebs, who thoughtfully made the parser for wrapfuncs flexible enough that `flags=flags|O_NOFOLLOW` would work. -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] [PATCH pseudo] ports/linux: wrap faccessat()
On Thu, 1 Aug 2019 18:02:06 +0200 Max Kellermann wrote: > + * wrap_access(int dirfd, const char *path, int mode, int flags) { This should probably say "faccessat". I know it's just a comment, but I try to be consistent about these. > + rc = real___fxstatat64(_STAT_VER, dirfd, path, , > AT_SYMLINK_NOFOLLOW); We should probably be using flags here, not AT_SYMLINK_NOFOLLOW. Or possibly (flags & AT_SYMLINK_NOFOLLOW). Otherwise, we'll get the wrong results if called with flags not including AT_SYMLINK_NOFOLLOW. -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] [PATCH] pseudo: Update to gain key bugfixes
On Sat, 13 Apr 2019 10:03:03 +0200 Andreas Müller wrote: > On Fri, Apr 12, 2019 at 10:18 AM > wrote: > > > > On Wed, 2019-04-10 at 18:59 -0500, Seebs wrote: > > > On Thu, 11 Apr 2019 00:12:54 +0100 > > > Richard Purdie wrote: > > > > > > > -SRCREV = "6294b344e5140f5467e6860f45a174440015304e" > > > > +SRCREV = "6ebc7d6bc8ab973d0ba949eeb363821811ce8dc5" > > > > > > I would sort of recommend one of the two commits since then -- one > > > may > > > fix the .NET startup failures, and the other fixes warnings that > > > this one introduced and I forgot to fix. But I'm also okay with > > > this one. > > > > That is good news. I've tested and updated to the later revision, > > thanks! > > > > Cheers, > > > > Richard > > > Hi, > > If you want a happy weekend - stop reading here. > > But somebody has to do widen the audience... > > Seems that this commit causes issues on meta-qt5 / qtbase [1]. To make > it worse it seems that only some host configurations are affected... > > Cheers > > [1] https://github.com/meta-qt5/meta-qt5/issues/187 > > Andreas My guess from looking at it is that qmake is using renameat2 and isn't falling back to something else if renameat2 fails with ENOSYS. -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] [PATCH] pseudo: Update to gain key bugfixes
On Thu, 11 Apr 2019 00:12:54 +0100 Richard Purdie wrote: > -SRCREV = "6294b344e5140f5467e6860f45a174440015304e" > +SRCREV = "6ebc7d6bc8ab973d0ba949eeb363821811ce8dc5" I would sort of recommend one of the two commits since then -- one may fix the .NET startup failures, and the other fixes warnings that this one introduced and I forgot to fix. But I'm also okay with this one. -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] [PATCH] Handle OFD lock flags
On Tue, 15 Jan 2019 13:31:13 +0100 Stefan Agner wrote: > + case F_OFD_GETLK: > + case F_OFD_SETLK: > + case F_OFD_SETLKW: These probably need an #ifdef for systems that don't have those names yet, but otherwise this looks reasonable? -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] [PATCH] pseudo: fix link of sqlite3 using pkg-config
On Sun, 18 Nov 2018 19:36:46 +0100 Jens Rehsack wrote: > + for sqlite_link_opt in $(pkg-config sqlite3 --libs --static) Ugh, but yeah, I think this is probably needed. I'm a bit concerned about libsqlite3 ending up with readline included, that really seems like something that should only be ending up in the sqlite3 binary, not in the library? None of this affects client side (libpseudo doesn't get linked against sqlite3), so it shouldn't cause problems for other programs. So this looks good to me. Sorry about the delay in getting to things, I am sorta swamped lately. -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] [pseudo] [RFC 1/2] pseudo_ipc.c: Use MSG_NOSIGNAL if available
On Wed, 15 Aug 2018 10:27:29 +0100 Richard Purdie wrote: > On Wed, 2018-08-15 at 08:59 +0200, Rasmus Villemoes wrote: > > On 2018-07-11 16:30, Rasmus Villemoes wrote: > > > MSG_NOSIGNAL has been in Linux since 2.2, and has been > > > standardized in > > > POSIX 2008. Using that when available avoids the overhead of the > > > two > > > syscalls to set and restore the SIGPIPE handler. Moreover, we can > > > eliminate one write() call by making use of sendmsg() to do > > > scatter-gather I/O. > > > > Ping. The README in the pseudo repo says to use the > > openembedded-core list for pseudo patches, so I assume this is the > > right place. I have other ideas for improving pseudo's performance > > on both client and server > > side, but there's no point sending patches to /dev/null. > > Pseudo maintenance is a little tricky right now and I'd guess Peter is > unavailable. Would you be able to format these as additional patches > to the pseudo recipe? That way we could test and include them in > builds and then Peter will review them when he has time. Hi! Sorry about the delay, I wanted to review these more carefully. They looked reasonable to me, I just haven't had a chance to look more closely. They're still in my "I need to respond to this" mailbox. -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] [PATCH] pseudo: use latest SRCREV
On Fri, 20 Apr 2018 08:25:26 + Martin Jansawrote: > * the pseudo.log is significantly shorter with this revision There's also an actually fairly significant bug fix in here. At one point, long ago, I made a function that expands paths, and takes a flag argument which may or may not be AT_SYMLINK_NOFOLLOW. And then for openat(2), it gets called with "flags", whatever those are. And it turns out that this means that any flag value other than 0 caused pseudo not to follow a terminal symlink when expanding path, which caused a number of very strange problems. And mostly those got corrected, but this should reduce errors. The change to drop the usually-harmless "path mismatch [%d links]" messages for N>1 is purely cosmetic, but we're talking about 4MB of cosmetic messages in a glibc locale build... -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] [PATCH][RFC] pseudo: intercept syscall() and return ENOTSUP for renameat2
On Wed, 4 Apr 2018 14:28:11 -0700 Khem Rajwrote: > what is the performance impact of adding another stack frame and > function call in the chain here. Do we have data ? Very close to unmeasurable, because *almost nothing ever uses syscall*. This is used only for the case where someone is explicitly calling syscall(), not for any other system call use case. And my implementation (which is not the same as this one) also overrides the wrapper generation, so there's no standard pseudo wrapper overhead (which is several times larger and involves mutexes and signal mask changing), it's just passing the call on unless it's SYS_renameat2. -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] [PATCH] pseudo: Upgrade to latest master
On Tue, 3 Apr 2018 10:51:28 +0100 Richard Purdiewrote: > This change includes several bug fixes and improvements, including > better path handling (the existance of . and .. for files), handling > of the sticky bit, and syscall renameat2 handling and interception > through syscall() which was breaking coreutils mv operations on > fedora27. > > [YOCTO #12594] > [YOCTO #12379] > [YOCTO #11643] Ack. This looks reasonable to me. The only change since then is I finally went back and changed the default copyright notice for new auto-generated guts files. :P Just to be clear, this does *not* handle/support renameat2; it rejects it with ENOSYS. Actually handling renameat2 correctly requires a change to the pseudo IPC protocol, because it needs a way to express an atomic exchange, which requires having two dev/inode pairs in the message. -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] [PATCH][RFC] pseudo: intercept syscall() and return ENOTSUP for renameat2
On Sat, 31 Mar 2018 16:03:01 -0500 Joshua Wattwrote: > Looks like maybe gdk-pixbuf-query-loaders has the unlucky honour of > being one of the few programs that invokes the pseudo syscall() > wrapper before any other functions that pseudo wraps and the missing > initializer made it explode? That seems exceedingly likely, because indeed, that's supposed to be at the top of the wrapper. Thanks. -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] [PATCH][RFC] pseudo: intercept syscall() and return ENOTSUP for renameat2
On Sat, 31 Mar 2018 14:12:55 +0100 Richard Purdiewrote: > I believe that libglib-2.0 does use syscall() for something and that > the above programs are calling into it and faulting. Interesting! I'll poke around and see what I can find. -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] [PATCH][RFC] pseudo: intercept syscall() and return ENOTSUP for renameat2
On Fri, 30 Mar 2018 23:02:29 -0700 Andre McCurdy <armccu...@gmail.com> wrote: > On Fri, Mar 30, 2018 at 10:06 PM, Seebs <se...@seebs.net> wrote: > > That's the problem: There's no sane way to express "the size that > > you would have gotten for these arguments of unknown types" > > And there I think lies the key point that you still haven't grasped. I think you're right, but also you're being sorta rude about it. I think you're missing the distinction between "broad enough experience with weird edge cases to be possibly-unduly cautious" and "has no idea which end of a compiler is up". > The arguments are not of unknown types - they are all of types which > are the same size as integer registers. It turns out, in C, "the same size" is *not enough information* to determine where an argument goes. It is probably enough information for the subset of arguments that syscall is using, on these architectures, but I say "probably" because I have no spec to point to that says it's necessarily the case. > The syscall manpage very > clearly documents the fact that all arguments to Linux syscalls are > passed to the kernel in registers. I think you even asked me why that > was important or useful. I'm still honestly not totally sure why it has to say *which* registers, specifically. Under what circumstances will my invocation of syscall(2) be changed by the knowledge that argument 5 to an OABI ARM system gets passed in v1? The information being present implies that I might need to know this to invoke syscall(2) correctly, but the only examples given are not actually specific to that information. > If there's any user space code out there which calls libc syscall() > and does not pass variables which libc syscall() (or a wrapper for it) > can unconditionally treat as a type which fits into integer registers > than it's a bug the caller. End of story. This raises an interesting point. The man page says that the dummy 0 argument is needed for SYS_readahead because of an alignment issue, to force the value into r2/r3. It does not say that the manual split of the 64-bit value into two halves is needed, it just does that. Is that actually required anyway, or only because of the need to pad the argument list? (For instance, look at sync_file_range2(), which reorders the arguments to sync_file_range() for efficiency. ... Oh, wow. I went and checked on readahead(): >> On some 32-bit architectures, the calling signature for this >> system call differs, for the reasons described in syscall(2). Note how they don't specify what the alternative calling signature is. -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] [PATCH][RFC] pseudo: intercept syscall() and return ENOTSUP for renameat2
On Fri, 30 Mar 2018 21:15:18 -0700 Andre McCurdywrote: > Arguments passed by the caller will be put on the stack before any > stack frame is created by the callee. You can argue about which way a > stack grows (up or down) but however you define it, reading past the > end of the arguments passed on the stack by the caller is never going > to read into the stack frame created by the callee, so this can't have > the intended affect. I definitely think it's Probably Purely Superstitious, but I'm not sure about this. I seem to recall at least one environment in which consecutive parameters had increasing addresses, and local variables had addresses higher still, so that somewhere past the address of the Nth argument would be the address of a local variable. Given how many insane calling conventions there are, I can't rule it out. (I do think you're right about the compiler probably optimizing it away, although they are not always as aggressive about that as you might expect them to be.) > This is probably going to work, but if the goal is to avoid reading > more from the stack than the generic C code would do, it doesn't > succeed. I'm aware. The purpose is to (1) use the thing most expressive of intent, (2) make sure that people know that this is not expected to be portable. "__builtin_apply()" is fairly clear as to its *intent*, and is unlikely to exist in a compatible calling convention in other compilers. > The "size" parameter to __builtin_apply() seems to simply > specify how much argument data to copy from the stack frame passed > byIt is not always simple to compute the proper value for size. the > caller. Setting it to sizeof(long long) * 7 is safe (ie it will copy > at least enough data from the stack frame passed by the caller, never > less) as it covers both the corner cases where registers are long > long (such as x32) and where _no_ arguments are passed in registers > and everything needs to be copied from the stack. However, on 32bit > targets (where registers are smaller than long long) and on any > target where _some_ arguments are passed via registers, it will cause > more data to be read from the stack than the generic C code would. Yes, it will. As the documentation says: "It is not always simple to compute the proper value for size." I do think this is currently too large; specifically, it looks as though right now there's a hard limit of 6 register-sized things, and anything that takes off_t or similar types has fewer than 6 arguments. So 6 * sizeof(off_t) or so is probably actually sufficient, if that stays true, but I don't see a feature test macro for it... > e.g. on 32bit ARM where the first 4 integer arguments are passed via > registers, the optimum value for __builtin_apply() "size" in order to > pass through 1 syscall number and 6 additional register sized > arguments would be sizeof(long) * 3. That seems probable, yes. > typedef long syscall_arg_t; /* fixme: wrong for x32 */ Yes. That's the problem: There's no sane way to express "the size that you would have gotten for these arguments of unknown types", so I intentionally went with something that may well be too long, but will not be too short. > (Note in the code was compiled with -mfloat-abi=soft to avoid > __builtin_apply() needing to save and restore all floating point > registers - which doesn't affect the amount of data read from the > stack, but makes the assembler more than twice as long...). And I don't *think* any syscalls take float arguments, but I don't know that this is not only true now, but going to stay true. -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] pseudo: host user contamination
On Thu, 29 Mar 2018 14:04:00 +0200 Enrico Scholzwrote: > __builtin_apply() should deal with it. The documentation hints at possible problems when dealing with other functions, but doesn't mention any architecture difficulties. It's possible gcc simply never uses any of the weird conventions, it's also possible that builtin_apply just doesn't work reliably on such architectures. The gcc docs do state that gcc's calling conventions never depend on whether a function has a fixed or variadic argument list, which suggests that it's probably safe-ish. (Some compilers use very different calling conventions for variadic functions.) -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] pseudo: host user contamination
On Tue, 27 Mar 2018 13:52:28 -0700 Andre McCurdy <armccu...@gmail.com> wrote: > On Tue, Mar 27, 2018 at 1:20 PM, Seebs <se...@seebs.net> wrote: > > My concern is that, strictly speaking, this is nearly all undefined > > behavior, and that reading more arguments than you were passed > > *does* explode on some C implementations. > Can you give some examples? Not specific ones off the top of my head, no. > For every architecture I'm aware of that supports Linux, reading more > arguments is going to mean reading more data out of the stack. It's > not going "explode" until you read far enough to reach beyond the > start of the stack. What other failure modes are there? There are weird calling conventions out there. For instance, "pass floating point values in registers, but integers on stack", or "pass first N arguments in registers", and so on. I don't know if any of them are active in stuff Linux supports, but I'm aware that this is an area where you can get really strange behaviors. It's undefined behavior for a reason. > ALL of the implementations of libc syscall() I've looked at in both > glibc and musl do BOTH of these things - either explicitly in C code > or effectively the same thing in assembler. Yes. > By trying to avoid them in a wrapper, you are holding yourself to a > higher standard than any of the underlying syscall() implementations. Well, yes. They're part of the implementation and can make assumptions about architecture because they're in a position to define architecture. I'm not part of the implementation, and I don't want to take on the workload of trying to track every possible architecture if there's any possible way I can avoid it. -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] pseudo: host user contamination
On Tue, 27 Mar 2018 13:12:19 -0700 Andre McCurdywrote: > If you mean forwarding arguments through a wrapper without > interpreting them then I don't know what your concern is. Forwarding > arguments can be handled completely generically - for any architecture > and any syscall. See the musl implementation. My concern is that, strictly speaking, this is nearly all undefined behavior, and that reading more arguments than you were passed *does* explode on some C implementations. Possibly none of the ones musl is targeting. I'm trying to minimize assumptions that *could in principle* affect portability, such as "it's safe to grab an arbitrary pool of arguments with va_arg", or "it's safe to grab arguments with va_arg using different parameter types than were used to store them". Because assumptions like those periodically break when, for some inexplicable reason, someone ports to an architecture that isn't a VAX 11/780. We're already stuck with "duplicating library functions" as a risk. But so far, I don't think I have any code which is manipulating arguments in a way that violates the spec. Adding such code creates an additional risk, however small that risk may be in practice right now. > However the good news is that code in a syscall() wrapper doesn't need > to be any *more* aware of argument ordering than the C code calling > syscall(). In this particular case, if the code in gnulib calling > syscall(SYS_renameat2, ...) doesn't do anything architecture specific > then either it's not needed (and therefore also not needed in a > syscall() wrapper which wants interpret renameat2 syscalls) or there's > a portability bug in gnulib. ie there is no case where architecture > specific awareness is required in a syscall() wrapper but not in the > original C code which calls syscall(). Yes. Right now, I think my inclination is to make a renameat2() wrapper which fails. We did that for renameat() originally, and it was years before it actually came up, and I think it's premature to attempt the wrapper at a time when I *can't* write test code which compares it to the behavior of libc. -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] pseudo: host user contamination
On Tue, 27 Mar 2018 21:20:24 +0200 Enrico Scholzwrote: > SYS_readahead is one of a few syscalls which pass 64 bit arguments on > 32 bit architectures. Without the manual splitting, the ABI will > cause the compiler to insert a dummy argument so that registers are > aligned for 64 bit values. I'm now even more confused. This sounds like the compiler *would* insert the argument without being told to, because the ABI "will cause" that, in which case the manual splitting wouldn't be necessary? -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] pseudo: host user contamination
On Tue, 27 Mar 2018 12:11:22 -0700 Andre McCurdywrote: > In the readahead example, the first syscall argument is the > 32bit file descriptor (which will be passed to the kernel in r0), > therefore a padding argument is required to fill r1 and ensure that > the first word of the 64bit offset gets passed in r2. Yes. > The above is completely specific to ARM 32bit EABI. I guess *similar* > issues may apply to some other 32bit architectures (as suggested in > the manpage). It's certainly not an issue with is generic to all 32bit > targets though. I was wondering about 64-bit EABI. The man page didn't say "32-bit EABI", it said "EABI". The information that you don't need to do that on at least some ARM EABI arguably makes this *worse*, rather than *better*, from the standpoint of "how do I write correct code for this". So this appears to be at least partially a documentation error, although it's quite possible that the text predates the question having come up. But it does also mean that it should be harmless to us in this case. > If syscall(), or a wrapper for it, *does* need to interpret the > arguments for a particular syscall then the syscall() implementation > would have to also agree with the interpretation of the data defined > by the kernel. Yes. My basic concern is that I don't think I have enough information to produce a Provably Correct handling for syscall arguments in the presence of at least one architecture where argument order can change for at least one syscall. ... That said, an actual *correct* wrapper for renameat2 turns out to be surprisingly hard, mostly because EXCHANGE is impossible to do with pseudo's current IPC data structure. -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] pseudo: host user contamination
On Tue, 27 Mar 2018 18:26:05 +0200 Enrico Scholzwrote: > Perhaps we have different man pages but e.g. [1] mentions only > registers in the context of the kernel interface but not when > entering/leaving syscall(2) itself. And yet, it's syscall(2) doing the thing... or possibly it's not, because it turns out I don't read IA64 assembly. > When, then this is completely undocumented and a glibc-only thing. > Other implementations[2] follow the behavior described in the man page > and do not set some magic registers on return. > I did not found the glibc syscall implementation for MIPS atm. Hmm. In MIPS, it does appear that glibc's syscall is *using* the return register, rather than writing to it. Yeah, I misread the IA64 code. I'm gonna go ahead and try to implement a wrapper. Until I've got test cases available, it's gonna be a custom wrapper, that will ENOTSUP for renameat2, and try to pass other things on naively. But I think Andre and Enrico are right that the code in glibc is not doing what I interpreted it as doing. -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] pseudo: host user contamination
On Tue, 27 Mar 2018 18:35:32 +0200 Enrico Scholzwrote: > Does this really matter here? Because the caller has to set them > accordingly the ABI, you can extract the arguments by > > int olddirfd= va_arg(ap, int); > char const *oldpath = va_arg(ap, char consr *); > int newdirfd= va_arg(ap, int); > char const *newpath = va_arg(ap, char consr *); > unsigned int flags = va_arg(ap, unsigned int); > > There are no 64 bit arguments (on 32 bit platforms) which might > require a special treatment as described in [1] > "Architecture-specific requirements". Okay, ignore the pointer case, and pretend it's the 64-bit value case, since we have specific-ish documentation for that. Look at the example for SYS_readahead, stating that the caller must pass an extra value. At that point, if you have a series of va_arg calls corresponding to the values that would have been arguments had they not passed the extra value, I don't think you get the expected arguments. So far as I can tell, if the caller actually wrote varargsfunc(SYS_readahead, 0, uint64_t_value, ...) and the function did va_arg(ap, uint64_t); they would not get the value passed as the third argument, because the calls to va_arg don't match the arguments passed. If you could just ignore this, the SYS_readahead example wouldn't have to exist; you could just follow the ABI and provide a 64-bit value. -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] pseudo: host user contamination
On Tue, 27 Mar 2018 16:42:03 +0200 Enrico Scholzwrote: > will work to wrap syscall(2). Params for _renameat2_syscall() can be > extracted by va_args. Does anyone have access to an actual 64-bit EABI ARM system to verify the argument passing for renameat2 there? > Code generated above is very ineffective; perhaps you can create > specialized assembly instructions which just jump into orig_syscall. I do not think I want to start adding assembly to pseudo, because I do not feel like learning assembly for all the architectures we currently run on. (Pseudo is, I believe, known to work across x86, PPC, MIPS, and ARM, in both 32-bit and 64-bit variants, also weird stuff like the x32 variant of x86.) -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] [PATCH][RFC] pseudo: intercept syscall() and return ENOTSUP for renameat2
On Tue, 27 Mar 2018 16:48:02 +0100 Ross Burtonwrote: > This patch intercepts syscall() and returns ENOTSUP if renameat2() is > being called. I am inclined to NAK this until we have a clearer understanding of the mechanics observed in glibc's syscall implementation; it's doing magic that this will not do, and will in fact undo, and we don't know *why* it does that magic. It seems dangerous to break a thing without first knowing why it was there in the first place. If we want to wrap this, at a bare minimum, we should be using a custom wrapper which doesn't use any of the standard wrapper magic. At that point, we can *probably* pass args along and just return immediately after the real_syscall call and get usable results? (And bail prematurely if it's a call we need to prevent.) -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] pseudo: host user contamination
On Tue, 27 Mar 2018 15:06:40 +0200 Enrico Scholzwrote: > Andre McCurdy > writes: > > >> Since the man page gave the ia64 example, I went and checked, and > >> it is indeed the case that calls other than syscall(2) will > >> clobber r10 after system calls, > > I think you are misinterpreting the man-page. In "Architecture > calling conventions" it documents the calling convention into the > kernel. syscall(2) itself is an ordinary function which has to > follow the userspace ABI; after jumping into the kernel and setting > 'errno' in error case, it restores registers as needed. I don't think this is what it's talking about. > Some ABIs allow functions to clobber registers (they are not restored > after leaving the function and do not carry a return value); e.g. on > ARM, these are r0-r3 and r12. That's probably the case for r10 in > ia64 too. Maybe you missed the previous message where I pointed out that this behavior is, at least on MIPS, an explicit step taken by glibc's syscall implementation (and many other system calls). So, no matter what the kernel's internal syscall behavior does, *after* the syscall has returned, glibc is checking whether a syscall returned -1, and setting a register based on that. This isn't a generic clobber; this is an explicitly specified value that the register shall have after the completion of the call, which glibc is implementing in code. And we don't actually know why, because as Andre has pointed out, if you don't do that, nothing obvious breaks in the test cases we've tried. (Admittedly, I don't think we've tried on any of the architectures where such a convention exists.) -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] pseudo: host user contamination
On Mon, 26 Mar 2018 19:59:09 -0700 Andre McCurdywrote: > The syscall manpage is from the kernel manpages, not glibc. > http://man7.org/linux/man-pages/man2/syscall.2.html And yet! glibc is setting those registers in its code. Why? If that's a kernel thing and libc doesn't need to do it, why is libc doing it? If it's "useful background information", what exactly is it "useful" for? > Personally, I've read the manpage, I've read code in glibc and musl, > I've straced coreutils mv and various little test programs on 32bit > ARM plus 32bit and 64bit x86 and written a wrapper for libc syscall() > which either intercepts or passes through syscalls. Okay, you've read the code in glibc and understand it. So, why does the glibc code have that register-setting assembly, if that register-setting assembly doesn't matter? You've told me several times that we don't need to think about the register-setting code. So why did glibc include it? > Everything I've > found seems to be consistent to the point that I've satisfied myself > that I have a pretty clear understanding of how libc syscall() works, > including why ARM EABI sometimes needs an extra argument to offset > 64bit values - and when it matters for a wrapper and when it doesn't. > I don't think there's much more I can do. Okay, you say you understand why ARM EABI "sometimes" needs an argument to offset things. What are the circumstances? Is it specific to 32-bit targets? On a target with 64-bit pointers, would it apply also to 64-bit pointers, or is it exclusively for 64-bit integers? Because it seems to me that on a 64-bit target, renameat2() would in fact be passing a 64-bit object as the second argument. And if there's a reason that this doesn't count as a 64-bit argument passed after an odd number of 32-bit arguments, I'd like to know specifically what that reason is before I go relying on it to stay true forever. -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] pseudo: host user contamination
On Mon, 26 Mar 2018 18:34:10 -0700 Andre McCurdywrote: > > I remain interested in why the glibc implementation does all these > > weird things on some architectures if none of those things matter. > > Which glibc implementation? I'll take a look. syscall(2) for various architectures, which is actually implementing all this fancy ABI stuff. If that doesn't matter, why's it there? I think we may be talking past each other. I'm not looking for "I tried this once on one architecture and it worked." I'm looking for a good enough understanding of *why* all these things are in the man page, and when they might matter, that I can reasonably predict whether this will work on lots of other platforms, and continue to work in the future. Pseudo is already way off in the weeds, but it mostly works, and the reason it mostly works is that I try to find out why things are the way they are rather than disregarding them. (And I'm thinking I should possibly add an is-syscall flag to wrappers, and then have those wrappers check returns and recreate the success/fail state right before actually returning.) -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] pseudo: host user contamination
On Mon, 26 Mar 2018 18:10:07 -0700 Andre McCurdywrote: > I've verified that the QA warnings on FC27 can be fixed by preloading > a wrapper for libc syscall(). Just a proof of concept but if anyone > would like to reproduce it my steps are below. Yes, I think we were expecting it would work on x86, where the ABI is trivial and friendly? I remain interested in why the glibc implementation does all these weird things on some architectures if none of those things matter. -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] pseudo: host user contamination
On Mon, 26 Mar 2018 13:12:44 -0700 Andre McCurdywrote: > That's based on your assumption that a C wrapper needs to care about > results in architecture specific registers, which I contend is not a > correct interpretation of the syscall manpage. My observation is: If this doesn't matter, why is glibc doing it? It seems really weird to mention this thing, and bother doing it, if it *never* matters. So possibly it does matter. Sometimes. When? I don't feel comfortable assuming I understand the code if it's doing something like that, I can't see when it would affect anything, and the code hasn't been removed to improve performance. I'd be a lot more comfortable disregarding the weird return values and register specifications if I could look at real-world examples of how that information is used. > Did you find any evidence to support your interpretation? e.g. Did you > find any examples of callers to the libc syscall() API which use > architecture specific assembler to examine the result of the syscall? I have seen exactly one use of syscall() in the wild at all, that being the recent addition to coreutils. The evidence for my interpretation that you *could* need to know about arch-specific behavior is the EABI example, which clearly indicates the *possibility* that code in C has to care about architectural variance in non-obvious ways. I don't know what ways those might be, and this call is so rarely used that I'm not sure it would be reasonable to generalize about it. (I also don't know whether it's still true on 64-bit ARM, and whether it also applies to pointer values or only to integer values, or...) > The gnulib code calling syscall(SYS_renameat2, ...) certainly doesn't > do that - it just checks the C function return value and errno. Since > there's no architecture specific code to examine the syscall() result, > do you expect coreutils mv to now incorrectly detect errors on ia64? No. But I don't know whether *anyone* is using syscall(), other than this one single example I've seen identified. I also don't know how widely tested the code in question is, or on what architectures. Testing on non-x86 architectures has often been sporadic in the open source community, and I think that's improved, but I don't know how much. If I'm looking at something that's almost never used, and I don't have specific information that the existing usage is being fully tested on "obscure" targets (such as mips, arm, etc), I am going to be at least a little distrustful. -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] pseudo: host user contamination
On Mon, 26 Mar 2018 20:49:30 +0200 Andreas Müllerwrote: > Interesting background: mv/renameat2 change seemed so important for > Fedora that they backported the changes into 8.27. It looks like the reason for this is the RENAME_NOREPLACE flag, which avoids a possible race condition. FWIW, I've traded a couple of emails with the coreutils people, and I think at this point I'm going to try a custom wrapper for syscall that just yields ENOTSUPP, because any attempt to do something fancier seems like it's going to be potentially error-prone. Since the man page gave the ia64 example, I went and checked, and it is indeed the case that calls other than syscall(2) will clobber r10 after system calls, so it's actually not possible for a C wrapper to do what we want on an intercepted syscall. Luckily for everyone, no one actually cares about ia64, aka Itanic. -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] pseudo: host user contamination
On Sat, 24 Mar 2018 15:22:47 -0500 Joshua Wattwrote: > I don't think that is true. libc's syscall() must conform to the *C* > ABI for the system... if the kernel does things that aren't in line > with the C ABI (like return things in registers that aren't expected, > fail to preserve registers that require preservation, or whatever), > wouldn't the libc syscall() be *required* to paper over it so that it > looks like a valid C call? Otherwise, it could never be safely called > from C code. I think this is only partially true. There's extra warnings in syscall(2) about weird kinds of non-conformance with the usual ABI, like the magic for 64-bit values on EABI (or possibly 32-bit EABI), and I think the point about the extra registers or possible register-smashing is just "at this point, you get the behavior of the actual syscall, which may violate the ABI." And yeah, it returns correctly, but if code's written to interact with what syscall() is *supposed* to do, and we trample that in some way, that's potentially bad. I honestly have no idea how much scope there is for weird problems, or whether I'm reading too much into the man page. But there's comments in the man page that seem like very things to say if libc's syscall really is just hiding all this complexity. Why on earth would the man page need to mention those things? What is their relevance, if the implementation covers all of that? Practical answer: I'm probably going to attempt the thing, with the first pass being (1) implement a wrapper for renameat2(), (2) implement a wrapper for syscall, (3) try to change syscall's behavior only in the case where the call is renameat2, (4) make this available and let people try it on a variety of architectures and hope for the best. -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] pseudo: host user contamination
On Sat, 24 Mar 2018 12:42:45 -0700 Andre McCurdywrote: > Right. The musl example is to show how it's possible to transparently > intercept and pass on any call to the syscall() ABI without > interpreting anything. Yes, if you don't need to interpret things, and aren't making additional other unrelated system calls after doing so. > Those details are all taken care of within the libc implementation of > syscall(). It's not something we need to care about at all in a > wrapper for it. I don't think that's correct. musl's call sequence: real_syscall() // sets a3 return pseudo's call sequence: various_setup() real_syscall() // sets a3 other system calls // also set a3 return In the case where pseudo is actually *disabled*, we just return right away after the real call. In every other case, we're making other calls some of which imply system calls, and those system calls could potentially overwrite things that the libc implementation of syscall took care of. (Mutex and signal mask operations.) So for that to work, I would in principle have to stash the value stored in, for instance, "a3", wait until after the other system calls, and then restore it. Unless *only* syscall() itself actually sets that register, and other system calls don't, and nothing else is using it either. -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] pseudo: host user contamination
On Sat, 24 Mar 2018 11:59:27 -0700 Andre McCurdywrote: > The EABI example applies to 64bit values on 32bit architectures. Since > pointers are 32bit values on 32bit architectures the example doesn't > apply to renameat2 (which only passes int's and pointers - nothing > which would be a 64bit value on a 32bit architecture). ie there is > never any "oldpathhi, oldpathlo", only "oldpath", etc. I didn't see a qualifier about it being only on a 32-bit architecture, it just says "EABI". But in general, this is the reason that musl's ability to work doesn't buy us guarantees; musl doesn't have to *interpret* the arguments. So for instance, they could just pass "the same arguments" for SYS_readahead, we couldn't. (If we needed it, which I don't think we do.) Similarly, they don't have to do Fancy Complicated Fixups around their system calls which can break weird register conventions. Consider: > >On a few architectures, a register is used to indicate > > simple boolean failure of the system call: ia64 > > uses r10 for this purpose, and mips uses a3. I have no evidence that these registers are being reliably saved through all the *other* code pseudo has in and around wrapper functions. By the time we get into wrap_foo(), we've already done a ton of other things, including made system calls, and we make *more* system calls on the way back out. So to handle that reliably, we'd need an extra special fancy wrapper function which bypasses everything for all the non-renameat2 cases, and even then, what would we do for renameat2? I can't write C code which stashes r10-on-ia64-or-a3-on-mips on its way out of the wrapper, then restores them after everything else is done. I can try writing the code, and it might well work, but I want to make it clear that this is a case where there's no guarantees at *all* that any code that can be written in remotely-sane C without assembly can actually do the thing correctly. The musl code is not trying to deal with a case where there's multiple other syscalls after the real syscall has been called, but before returning. This is not "we definitely can't", this is warning that there's reason to think it may not work, or may require a great deal more magic than anything else does. > If there _were_ some architecture dependent ordering of the renameat2 > arguments passed into the syscall() ABI then it would have to be > reflected in the code which originally calls syscall(), e.g. the code > in gnulib. Yeah. But about all my crystal ball will tell me about possible future changes or syscalls or ABI choices is that there might be some. -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] pseudo: host user contamination
On Sat, 24 Mar 2018 11:12:20 -0700 Andre McCurdywrote: > If you can successfully intercept the libc syscall() API and return > ENOTSUPP for the one specific case of renameat2 but pass on all other > callers transparently then haven't you've already solved the bulk of > the problem (for the non-Go case)? > Or are you suggesting unconditionally returning ENOTSUPP for every > syscall called via the libc syscall() API? I have no idea what happens if we try to ENOTSUPP things. I would be wary of it. > It's basically exactly what the musl syscall() wrapper does. ie fetch > 6 register sized vaargs values from the caller and pass them on in the > same order to the next syscall(). Yes. That might well work in most cases? > None of that matters if you don't need to interpret the arguments - > you just need to pass them on in the same order you received them. That is not necessarily true in every ABI. But I'm more concerned about the comments about *returns* and special register usage. > I don't see any evidence to support all this doom and gloom, but if > there is a corner case which fails then it will also fail when running > on musl - so at least you won't be debugging it on your own :-) Possibly? I've never encountered musl, and don't know how broad their coverage is. But I'm put in mind of a quote from some years back: "This guy I know says you can't just carry the ball in basketball, but I got a basketball and tried it, and it worked fine." I'm not saying this will necessarily fail immediately. I'm saying there's nothing even *like* a guarantee that it will work, or that it will keep working. And I am concerned about the fairly unbounded possible risk cases. Every other function we wrap *has* a meaningful prototype, with arguments in a knowable order. But look at the EABI example; if we want to actually *process* renameat2(), it's not enough to pass arguments on blindly, we have to be sure we know exactly which arguments are which. If the pointers can be 64-bit pointers, though, that may mean that we have to expect the incoming arguments to be (olddirfd, 0, oldpathhi, oldpathlo, newdirfd, 0, newpathhi, newpathlo, flags), but only on some architectures, where others would use (olddirfd, oldpath, newdirfd, newpath, flags). Do we have any 64-bit pointer EABI? Heck if I know, I don't track that, because this is the only time I can think of where I'd have had to. > Since the libc syscall() API is only going to be used for syscalls > which don't have an libc wrapper, I'm not sure I'd bet on that. If it exists, people will sometimes use it, and sometimes use it poorly. -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] pseudo: host user contamination
On Sat, 24 Mar 2018 17:10:47 + "Burton, Ross"wrote: > On 24 March 2018 at 12:36, Richard Purdie > wrote: > > I think, at least in principle, pseudo could wrap that and intercept > > this particular syscall, check syscall_number (the numbering having > > its own set of issues) and then only handle the specific problem > > case we have. > > And to make things easier I think we could even just ENOTSUPP > renameat2 in the short term (i.e. for 2.5), before looking at a more > comprehensive intercepting > which could solve the Go issue. In the Go case, we would basically have to do something more like debugger traps. They're not using libc *at all*, and unless something's built with cgo or requires C-type libraries, it's not even going to be dynamically linked. No dynamic linker => LD_PRELOAD is irrelevant. > I filed a bug with coreutils yesterday. "Just intercept syscall()" > they said. If they can describe a mechanism for intercepting syscall that they can guarantee will work across all Linux architectures including possible future architectures not yet in use, I'd love to know what it is. See syscall(2) for some examples of the kinds of things that could be concerns, such as the EABI calling convention. We can sort of hope for the best if we just treat everything as a chain of unsigned longs, but that's really *not* safe, and it should not be expected to work reliably across architectures. Honestly, reading it more closely, I don't think we can actually produce behavior that precisely mimics the behavior of syscall() for generic cases on architectures we currently run on. There's magic like setting values in other registers, clobbering registers, and so on, because *this function does not obey general architecture calling conventions*. And if the wrapper does, the wrapper will break at least some of the expected behaviors, by not behaving the same way. Basically: I don't think we can promise that we will correctly pass through both parameters to syscall() and returns from it in on existing architectures we're actually running on today, for the whole set of possible syscalls. So if we intercept syscall(), at least some previously-valid programs break. -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] pseudo: host user contamination
On Sat, 24 Mar 2018 12:36:28 + Richard Purdiewrote: > I think, at least in principle, pseudo could wrap that and intercept > this particular syscall, check syscall_number (the numbering having > its own set of issues) and then only handle the specific problem case > we have. I think the problem is the lack of a generic mechanism for "oops nevermind just pass the arguments along to the child". There's actually a neat post in Go land pointing out a mechanism for a somewhat-similar circumstance in which their compiler just plain cheats, and does not actually do the full function call setup, just leaves the stack in a place that works *as if* the parent had called something else. > The unfortunate reality is we will have to figure out some solution to > this as f27 is in the wild now. Explaining why this causes problems > for debian/yocto to the upstream is also obviously a good idea. I would like to put in a (weak, and i know it's impractical) vote for just labeling this a host system requirement, "don't use versions of coreutils that jumped to a not-yet-fully-supported new system call." -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] pseudo: host user contamination
On Fri, 23 Mar 2018 18:43:12 -0700 Andre McCurdywrote: > Since glibc doesn't provide a wrapper for renameat2, making the > syscall via the libc syscall() API is exactly what coreutils (actually > gnulib) should be doing. There would certainly be grounds to complain > if user space code were making a syscall directly, but that's not > what's happening here - the syscall is still being made from within > libc. Ahh, okay, that clears it up some. I thought coreutils was making a direct invocation of a syscall rather than using the generic syscall wrapper. I guess my thought is that if glibc isn't providing a wrapper for a syscall, it's probably best to avoid that syscall unless it's impossible to run without it. And since I *think* someone may have implemented mv(1) in the past without renameat2(), it seems to me that switching it to use a new syscall that libc doesn't have a wrapper for yet is perhaps premature. There's been a lot of issues we've run into that were caused by coreutils or something near it being very excited about switching to a new, and preferably larger and more complicated, API. Usually to do something that was already being done successfully and without problems; for instance, the use of posix ACL xattrs to encode standard permission bits even when no other ACL functionality is in use. I am not positively impressed by this; if what you're doing can be done entirely with a stable interface that hasn't changed in the last decade or two, switching to a newer interface seems sort of counterproductive. I have some concerns about the API for syscall(), and there's a lot of behavior here that's potentially more-undefined-than-usual; for instance, using va_arg to get more arguments than were actually passed. But it may be we don't really have a way around it. -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] pseudo: host user contamination
On Fri, 23 Mar 2018 18:10:21 -0700 Andre McCurdywrote: > The syscall wrapper in musl handles 6 additional arguments - > unconditionally. Based on that you might not need to interpret > anything - just extract 6 arguments and pass them on? > > long syscall(long n, ...) > { > va_list ap; > syscall_arg_t a,b,c,d,e,f; > va_start(ap, n); > a=va_arg(ap, syscall_arg_t); > b=va_arg(ap, syscall_arg_t); > c=va_arg(ap, syscall_arg_t); > d=va_arg(ap, syscall_arg_t); > e=va_arg(ap, syscall_arg_t); > f=va_arg(ap, syscall_arg_t); > va_end(ap); > return __syscall_ret(__syscall(n,a,b,c,d,e,f)); > } That is the sort of thing which *might* work, but which is potentially subject to arch-specific calling conventions or strangeness. It's worth a try, I guess? But I also think it may be worth just having all the people maintaining stuff that expects this go yell at coreutils about bad implementation choices, like "bypass libc to make raw syscalls when you are not, in fact, implementing libc". -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] pseudo: host user contamination
On Fri, 23 Mar 2018 17:33:28 -0700 Andre McCurdywrote: > Interposing the libc syscall wrapper doesn't seem to scary if you can > transparently pass on everything apart from syscall(SYS_renameat2, > ...) ? I don't think I can in the general case; I don't know how many arguments every possible syscall takes. -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] pseudo: host user contamination
On Fri, 23 Mar 2018 23:47:30 + Richard Purdie <richard.pur...@linuxfoundation.org> wrote: > On Fri, 2018-03-23 at 11:49 -0500, Seebs wrote: > > On Fri, 23 Mar 2018 16:30:55 + > > "Burton, Ross" <ross.bur...@intel.com> wrote: > > > > > > > > Because in GNU's infinite wisdom they're using renameat2() to do > > > atomic renames in the mv command, and as renameat2 isn't in the > > > headers for F27 it just does a syscall directly. This is in > > > upstream > > > coreutils so once they make a release, everyone gets it. > > UGH. > > > > I... am really unsure whether it's possible to catch that, because > > I really, really, don't want to try to intercept raw syscall() > > calls. I don't think that ends well. > > Just out of interest for my education, why is that a really bad idea? > Loops, e.g. with memory allocation issues? Potentially. We rely pretty heavily on the assumption that an *actual* syscall can go through. Although... Actually, I don't even know if this is an actual syscall. This could be an actual glibc wrapper around the syscall interface, just like all the others, which is not the *actual* raw syscall or whatever, and... I have no idea how often that is or isn't hit. It's totally possible it would work, but basically, I have a pretty good intuition of when something sounds brittle and error-prone, and trying to trap syscall() sounds brittle and error-prone and might work today but not next week... -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] pseudo: host user contamination
On Fri, 23 Mar 2018 16:56:16 + "Burton, Ross" <ross.bur...@intel.com> wrote: > On 23 March 2018 at 16:49, Seebs <se...@seebs.net> wrote: > > On Fri, 23 Mar 2018 16:30:55 + > > "Burton, Ross" <ross.bur...@intel.com> wrote: > > > >> Because in GNU's infinite wisdom they're using renameat2() to do > >> atomic renames in the mv command, and as renameat2 isn't in the > >> headers for F27 it just does a syscall directly. This is in > >> upstream coreutils so once they make a release, everyone gets it. > > > > UGH. > > > > I... am really unsure whether it's possible to catch that, because > > I really, really, don't want to try to intercept raw syscall() > > calls. I don't think that ends well. > > > > I wonder if they can be persuaded to, you know, NOT use a syscall > > directly when it's not in the system headers, on the grounds that > > the system headers define the exported interface, and bypassing > > them is almost certainly a very bad idea. > > Just chatting to the fakeroot maintainer now, as this is presumably > going to break the entire Debian build infrastructure when they get > the coreutils upgrade. He isn't massively thrilled either. They have > the option of just reverting these changes to coreutils though. It's *possible* that there's a workaround, but I think realistically the right answer is probably "yell at coreutils not to do that". -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] pseudo: host user contamination
On Fri, 23 Mar 2018 16:30:55 + "Burton, Ross"wrote: > Because in GNU's infinite wisdom they're using renameat2() to do > atomic renames in the mv command, and as renameat2 isn't in the > headers for F27 it just does a syscall directly. This is in upstream > coreutils so once they make a release, everyone gets it. UGH. I... am really unsure whether it's possible to catch that, because I really, really, don't want to try to intercept raw syscall() calls. I don't think that ends well. I wonder if they can be persuaded to, you know, NOT use a syscall directly when it's not in the system headers, on the grounds that the system headers define the exported interface, and bypassing them is almost certainly a very bad idea. -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] pseudo: host user contamination
On Fri, 23 Mar 2018 17:10:35 +0100 Enrico Scholzwrote: > I think, 'mv' is the culprit. It calls 'renameat2()' directly over > 'syscall()': > > | $ ltrace mv foo bar > | ... > | syscall(316, 0xff9c, 0x7fff1564a341, > 0xff9c)= 0 > > > Perhaps, 'pseudo' does not catch this? Yeah. And so far as I know, it's not actually *possible* to in the general case. I really don't think it's safe to try to catch syscall(). I was afraid someone would do this. (It also breaks most Go programs, for similar reasons; no libc calls.) I have no idea why they're doing that; it seems distinctly unsafe. -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] [PATCH] pseudo: explicitly enable xattr support
On Thu, 22 Mar 2018 17:06:08 +0100 Andreas Kaufmannwrote: > Pseudo is using a custom configure script that detects if it shall > build with extended file attribute support or not. The check is done > by simply calling 'getfattr' provided by attr-native which is not > part of the dependency list. Due to the recent changes (recipe > specific sysroot & cleanup of $PATH) this call fails now when the > recipe is being build for the first time (at least when being build > for nativesdk case). Explicitly setting up a dependency to > attr-native just to satisfy configure would be wrong also since the > real dependency is to attr/nativesdk-attr which are already part of > the dependency list (see DEPENDS). Therefore bypass the test in the > configure by explicitly enabling xattr using a configure option > available in any case. This seems reasonable to me. The historical rationale for the configure test was that we encountered actual systems in the wild which did not have the underlying xattr support, as I recall? But I think that's no longer remotely relevant. -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] [PATCH] pseudo: update to latest master
On Thu, 1 Mar 2018 15:53:46 +0200 Alexander Kanavinwrote: > toomanyfiles.patch rebased Do we still have a reason for that, if we're using epoll to address that? That said, with the new fastop sanity-fix, that is probably much less dangerous than it used to be. -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] [PATCH] pseudo: update to latest master
On Mon, 19 Feb 2018 20:01:31 +0100 Martin Jansawrote: > I did quick check for UID/GID issue: > https://bugzilla.yoctoproject.org/show_bug.cgi?id=12434 Hmm. glibc-locale, in particular, is a really weird case; does it still do the strange thing with copying the files around through scratch space to get them from one package to another? (My vague memory from five years ago is staying vague.) -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] [PATCH] pseudo: update to latest master
On Mon, 19 Feb 2018 11:27:56 +0200 Alexander Kanavinwrote: > > Huh. It's possible that the initial "don't try to close fd 0" was > > correct, and the real problem is that the attempt is getting made > > mistakenly. I'll study that more; the epoll code was a contribution > > and I may not have fully understood it. > > To be honest, it would have been better to apply my epoll patch as it > is, and then do additional modifications as separate commits. That > would make it simpler to isolate the issue. We've used my epoll patch > for many months without problems on the autobuilder and elsewhere. ... Wow, you know, now that you *mention* it, that is a really good idea. *sigh* Sorry about that. Hmm. > if (clients[events[i].data.u64].fd == listen_fd) { Just a sanity-check: Should this be equivalent to: if (events[[i].data.u64 == 0) ? The reason I ask is that, looking at the code, we should never, ever, be getting into close_client(0). The "<=" check was right. The only call to close_client anywhere in the epoll case is: > } else { > int n = 0; > ioctl(clients[i].fd, FIONREAD, ); > if (n == 0) { > close_client(i); > } else { > serve_client(i); > } And that's the else for clients... oh hey > if (clients[events[i].data.u64].fd == listen_fd) { ... > ioctl(clients[i].fd, FIONREAD, ); do you see the error? I do. This gets back to "and one of the problems with testing is that if I don't actually check the logs, I often don't see problems", because pseudo does enough internal disaster recovery that things can explode horribly without observable failures. Now extracting the data.u64 value and using that consistently as the index. Pushed fix to master. -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] [PATCH] pseudo: update to latest master
On Sat, 17 Feb 2018 15:28:55 +0200 Alexander Kanavinwrote: > For me, with epoll enabled: > > b6a015a works > 23f089f and 26e30fa both lock up Huh. It's possible that the initial "don't try to close fd 0" was correct, and the real problem is that the attempt is getting made mistakenly. I'll study that more; the epoll code was a contribution and I may not have fully understood it. -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] [PATCH] pseudo: update to latest master
On Fri, 16 Feb 2018 20:11:48 +0100 Martin Jansawrote: > I didn't get to the logs yet, but with this change added I see many > do_install tasks failing with exit code '134'. Huh, that's SIGABRT. I'll see if I can reproduce. -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] [PATCH] pseudo: update to latest master
On Fri, 16 Feb 2018 17:55:54 +0100 Martin Jansa <martin.ja...@gmail.com> wrote: > $ grep -c "tried to close client 0 (highest is 1)" > update-rc.d/0.7-r5/pseudo/pseudo.log > 579655922 > > All pseudo processes I've seen on various servers which were building > with this change included got stuck until disk space run out.. Oh-hoh! That could be the epoll thing that I wasn't able to reproduce. > commit 26e30fa2e1a0fe4e885d7eea3f55d23cd2c3158f > Author: Seebs <se...@seebs.net> > Date: Fri Feb 16 11:49:49 2018 -0600 > > Allow closing client 0 Added a thing to master. It looks like the failure mode is that a client ended up on fd 0, and for some reason, back in 2010, I thought that would be invalid and did not allow it. So change is to replace "<= 0" with "< 0". -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] [PATCH][PSEUDO 0/4] Fix stripped mode with newer coreutils cp
On Wed, 29 Nov 2017 12:02:38 -0600 Richard Tollertonwrote: > So we noticed that cp -Rp was stripping special bits off of > directories when run under pseudo. The super interesting thing was > that people saw this when upgrading their build machines from Ubuntu > 12.04 to 16.04. Root cause is in the commit message in the first > patch, which also contains the fix. > > I also added a test for this, and fixed up the perms on another test. > run_tests.sh reports 13/13 pass. > > Also updated contact info in the README. This looks good to me. Thanks! (I'm still hoping to get a free day or two to merge patches. Any day now.) -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] [PATCH 2/2] pseudo: Add fastop reply fix
On Fri, 22 Sep 2017 17:41:17 +0100 Richard Purdiewrote: > This changes the pseudo FASTOP functionality so that a reply to the > operation is required. This means we then cannot lose data if a > connection is closed. This in turn stops corruption if we run out of > file handles and have to close connections. > > This tweaks the connection closure patch to update the comment there > which is now outdated. > > Signed-off-by: Richard Purdie This looks reasonable to me. I did some testing with a very similar patch and concluded that, while it slowed performance slightly, it didn't slow it nearly as much as the pre-fastop behavior did. -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] [PATCHv2] pseudo: use epoll API on Linux
On Mon, 18 Sep 2017 19:41:03 -0400 Trevor Woernerwrote: > Would you be open to a co-maintainership? Maybe a co-maintainer who > doesn't push to master directly? Maybe? I don't know, I'd need to think about that a bit. I recognize that there's a need to get some of this stuff fixed, but I think the amount of busy I've been recently is probably transient and not likely to happen very often. -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] [PATCHv2] pseudo: use epoll API on Linux
On Mon, 18 Sep 2017 19:16:13 -0400 Trevor Woernerwrote: > It doesn't make sense to carry these large patches against pseudo in > OE itself. Isn't pseudo one of the tools under The Yocto Project > umbrella? Yes. But the pool of people with the variety of expertise needed to maintain it isn't huge, and *normally* I'm more responsive, I'm just extra busy the last couple of months. -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] [pseudo][PATCH] Fix to fcntl guts to ignore flags that can be ORed into cmd
On Sat, 16 Sep 2017 02:22:01 + Will Pagewrote: > Thanks, Ross. > > I had contacted the maintainer off list to ask where I should submit > my patch. I may have misunderstood his reply. No, I'm just confused, because I'm not really otherwise active in yocto/oe-core these days, I just still know my way around pseudo. (And I know I'm several patches behind on things, but I'm really hoping to get some free time in a couple of weeks.) -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] [pseudo][PATCH] Fix to fcntl guts to ignore flags that can be ORed into cmd
On Fri, 15 Sep 2017 15:27:00 -0700 Will Pagewrote: > The fcntl guts switch on "cmd" parameter to identify the fcntl > command being issued, but isn't aware of the file creation flags that > can be ORed in. This is true. I was, in fact, not aware of those flags. Looks reasonable. -s -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core