Re: [OE-core] [PATCH RFC] bitbake.conf/pseudo: Switch from exclusion list to inclusion list

2023-12-12 Thread Seebs
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'

2023-05-30 Thread Seebs
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

2023-05-30 Thread Seebs
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

2022-06-27 Thread Seebs
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

2022-04-25 Thread Seebs
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

2022-04-25 Thread Seebs
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

2021-09-09 Thread Seebs
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()

2021-09-09 Thread Seebs
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

2021-09-09 Thread Seebs
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

2021-08-09 Thread Seebs
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

2021-07-29 Thread Seebs
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

2021-07-28 Thread Seebs
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

2021-07-28 Thread Seebs
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

2021-07-27 Thread Seebs
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

2021-07-27 Thread Seebs
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

2021-07-27 Thread Seebs
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()

2021-07-27 Thread Seebs
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

2021-07-27 Thread Seebs
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

2021-01-08 Thread Seebs
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

2020-09-29 Thread Seebs
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

2020-09-28 Thread Seebs
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

2020-09-28 Thread Seebs
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

2020-09-22 Thread Seebs
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

2020-09-22 Thread Seebs
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

2020-06-26 Thread Seebs
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

2020-06-26 Thread Seebs
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

2020-06-26 Thread Seebs
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

2020-05-08 Thread Seebs
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

2020-05-03 Thread Seebs
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

2020-05-02 Thread Seebs
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

2020-04-27 Thread Seebs
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

2020-04-09 Thread Seebs
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

2020-04-08 Thread Seebs
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

2020-04-04 Thread Seebs
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

2019-12-18 Thread Seebs
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

2019-12-18 Thread Seebs
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

2019-12-18 Thread Seebs
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

2019-11-11 Thread Seebs
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

2019-11-09 Thread Seebs
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

2019-11-09 Thread Seebs
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

2019-11-07 Thread Seebs
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]

2019-08-03 Thread Seebs
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]

2019-08-02 Thread Seebs
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]

2019-08-02 Thread Seebs
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]

2019-08-01 Thread Seebs
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

2019-08-01 Thread Seebs
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()

2019-08-01 Thread Seebs
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

2019-04-13 Thread Seebs
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

2019-04-10 Thread Seebs
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

2019-01-15 Thread Seebs
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

2018-11-18 Thread Seebs
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

2018-08-15 Thread Seebs
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

2018-04-20 Thread Seebs
On Fri, 20 Apr 2018 08:25:26 +
Martin Jansa  wrote:

> * 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

2018-04-04 Thread Seebs
On Wed, 4 Apr 2018 14:28:11 -0700
Khem Raj  wrote:

> 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

2018-04-03 Thread Seebs
On Tue,  3 Apr 2018 10:51:28 +0100
Richard Purdie  wrote:

> 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

2018-03-31 Thread Seebs
On Sat, 31 Mar 2018 16:03:01 -0500
Joshua Watt  wrote:

> 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

2018-03-31 Thread Seebs
On Sat, 31 Mar 2018 14:12:55 +0100
Richard Purdie  wrote:

> 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

2018-03-31 Thread Seebs
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

2018-03-30 Thread Seebs
On Fri, 30 Mar 2018 21:15:18 -0700
Andre McCurdy  wrote:
> 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

2018-03-29 Thread Seebs
On Thu, 29 Mar 2018 14:04:00 +0200
Enrico Scholz  wrote:
> __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

2018-03-27 Thread Seebs
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

2018-03-27 Thread Seebs
On Tue, 27 Mar 2018 13:12:19 -0700
Andre McCurdy  wrote:

> 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

2018-03-27 Thread Seebs
On Tue, 27 Mar 2018 21:20:24 +0200
Enrico Scholz  wrote:

> 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

2018-03-27 Thread Seebs
On Tue, 27 Mar 2018 12:11:22 -0700
Andre McCurdy  wrote: 
> 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

2018-03-27 Thread Seebs
On Tue, 27 Mar 2018 18:26:05 +0200
Enrico Scholz  wrote:

> 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

2018-03-27 Thread Seebs
On Tue, 27 Mar 2018 18:35:32 +0200
Enrico Scholz  wrote:

> 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

2018-03-27 Thread Seebs
On Tue, 27 Mar 2018 16:42:03 +0200
Enrico Scholz  wrote:

> 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

2018-03-27 Thread Seebs
On Tue, 27 Mar 2018 16:48:02 +0100
Ross Burton  wrote:

> 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

2018-03-27 Thread Seebs
On Tue, 27 Mar 2018 15:06:40 +0200
Enrico Scholz  wrote:

> 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

2018-03-26 Thread Seebs
On Mon, 26 Mar 2018 19:59:09 -0700
Andre McCurdy  wrote:

> 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

2018-03-26 Thread Seebs
On Mon, 26 Mar 2018 18:34:10 -0700
Andre McCurdy  wrote:

> > 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

2018-03-26 Thread Seebs
On Mon, 26 Mar 2018 18:10:07 -0700
Andre McCurdy  wrote:

> 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

2018-03-26 Thread Seebs
On Mon, 26 Mar 2018 13:12:44 -0700
Andre McCurdy  wrote:

> 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

2018-03-26 Thread Seebs
On Mon, 26 Mar 2018 20:49:30 +0200
Andreas Müller  wrote:

> 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

2018-03-24 Thread Seebs
On Sat, 24 Mar 2018 15:22:47 -0500
Joshua Watt  wrote:

> 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

2018-03-24 Thread Seebs
On Sat, 24 Mar 2018 12:42:45 -0700
Andre McCurdy  wrote:

> 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

2018-03-24 Thread Seebs
On Sat, 24 Mar 2018 11:59:27 -0700
Andre McCurdy  wrote:

> 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

2018-03-24 Thread Seebs
On Sat, 24 Mar 2018 11:12:20 -0700
Andre McCurdy  wrote:

> 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

2018-03-24 Thread Seebs
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

2018-03-24 Thread Seebs
On Sat, 24 Mar 2018 12:36:28 +
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.

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

2018-03-23 Thread Seebs
On Fri, 23 Mar 2018 18:43:12 -0700
Andre McCurdy  wrote:

> 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

2018-03-23 Thread Seebs
On Fri, 23 Mar 2018 18:10:21 -0700
Andre McCurdy  wrote:

> 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

2018-03-23 Thread Seebs
On Fri, 23 Mar 2018 17:33:28 -0700
Andre McCurdy  wrote:

> 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

2018-03-23 Thread Seebs
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

2018-03-23 Thread Seebs
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

2018-03-23 Thread Seebs
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

2018-03-23 Thread Seebs
On Fri, 23 Mar 2018 17:10:35 +0100
Enrico Scholz  wrote:

> 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

2018-03-22 Thread Seebs
On Thu, 22 Mar 2018 17:06:08 +0100
Andreas Kaufmann  wrote:

> 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

2018-03-01 Thread Seebs
On Thu,  1 Mar 2018 15:53:46 +0200
Alexander Kanavin  wrote:

> 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

2018-02-19 Thread Seebs
On Mon, 19 Feb 2018 20:01:31 +0100
Martin Jansa  wrote:

> 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

2018-02-19 Thread Seebs
On Mon, 19 Feb 2018 11:27:56 +0200
Alexander Kanavin  wrote:

> > 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

2018-02-17 Thread Seebs
On Sat, 17 Feb 2018 15:28:55 +0200
Alexander Kanavin  wrote:

> 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

2018-02-16 Thread Seebs
On Fri, 16 Feb 2018 20:11:48 +0100
Martin Jansa  wrote:

> 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

2018-02-16 Thread Seebs
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

2017-11-29 Thread Seebs
On Wed, 29 Nov 2017 12:02:38 -0600
Richard Tollerton  wrote:

> 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

2017-09-22 Thread Seebs
On Fri, 22 Sep 2017 17:41:17 +0100
Richard Purdie  wrote:

> 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

2017-09-18 Thread Seebs
On Mon, 18 Sep 2017 19:41:03 -0400
Trevor Woerner  wrote:

> 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

2017-09-18 Thread Seebs
On Mon, 18 Sep 2017 19:16:13 -0400
Trevor Woerner  wrote:

> 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

2017-09-16 Thread Seebs
On Sat, 16 Sep 2017 02:22:01 +
Will Page  wrote:

> 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

2017-09-15 Thread Seebs
On Fri, 15 Sep 2017 15:27:00 -0700
Will Page  wrote:

> 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


  1   2   >