[PATCH 1/4] staging: Ensure cleaned dependencies are added

[PATCH 1/4] staging: Ensure cleaned dependencies are added

[Richard Purdie]

Most recipe-sysroot dependencies are handled by these prefuncs. "configure"
is special with a decidated task, prepare_recipe_sysroot which runs
beforehand.

do_prepare_recipe_sysroot does not have to be run before/after
fetch/unpack/patch, they're independent tasks. If fetch/unpack/patch
have sysroot dependencies and those tasks rerun, stale items from the
sysroot could be uninstalled and since prepare_recipe_sysroot doesn't
re-run, they could be missing by the time configure runs.

Fix this by adding the prefunc for do_configure which would ensure
the sysroot is in a good state.

This issue can be reproduced by modifying do_unpack and watching
binutils-native and vulkan-headers failing due to missing makeinfo
or ninja.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 meta/classes/staging.bbclass | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta/classes/staging.bbclass b/meta/classes/staging.bbclass
index de3a19815a3..7c9fb38607e 100644
--- a/meta/classes/staging.bbclass
+++ b/meta/classes/staging.bbclass
@@ -614,7 +614,7 @@ python staging_taskhandler() {
     bbtasks = e.tasklist
     for task in bbtasks:
         deps = d.getVarFlag(task, "depends")
-        if deps and "populate_sysroot" in deps:
+        if task == "do_configure" or deps and "populate_sysroot" in deps:
             d.appendVarFlag(task, "prefuncs", " extend_recipe_sysroot")
 }
 staging_taskhandler[eventmask] = "bb.event.RecipeTaskPreProcess"
-- 
2.25.1

[PATCH 2/4] pseudo: Ignore mismatched inodes from the db

[Richard Purdie]

Currently, where pseudo finds a database entry for an inode but the path
doesn't match, it reuses that database entry metadata. This is causing
real world "corruption" of file attributes.

See [YOCTO #14057] for an example of this.

This can happen when files are deleted outside of pseudo context and the
inode is reused by a new file which pseduo then "sees".

Its possible the opposite could happen, it needs to reuse attributes
but this change would prevent it. As far as I can tell, we don't want
pseuo to reuse these attributes though so this code should be safer
and avoid bugs like the above.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 .../pseudo/files/delete_mismatches.patch      | 51 +++++++++++++++++++
 meta/recipes-devtools/pseudo/pseudo_git.bb    |  1 +
 2 files changed, 52 insertions(+)
 create mode 100644 meta/recipes-devtools/pseudo/files/delete_mismatches.patch

diff --git a/meta/recipes-devtools/pseudo/files/delete_mismatches.patch b/meta/recipes-devtools/pseudo/files/delete_mismatches.patch
new file mode 100644
index 00000000000..6c78d787c78
--- /dev/null
+++ b/meta/recipes-devtools/pseudo/files/delete_mismatches.patch
@@ -0,0 +1,51 @@
+When we see cases where the inode no longer matches the file path, pseudo 
+notices but currently reuses the database entry. This can happen where for
+example, a file is deleted and a new file created outside of pseudo where
+the inode number is reused.
+
+Change this to ignore the likely stale database entry instead. We're
+seeing bugs where inode reuse for deleted files causes permission corruption.
+(See bug #14057 for example). We don't want to delete the database entry
+as the permissions may need to be applied to that file (and testing shows
+we do need the path matching code which handles that).
+
+I appreciate this should never happen under the original design of pseudo
+where all file accesses are monitored by pseudo. The reality is to do that,
+we'd have to run pseudo:
+
+a) for all tasks
+b) as one pseudo database for all of TMPDIR
+
+Neither of these is realistically possible for performance reasons.
+
+I believe pseudo to be much better at catching all accesses than it
+might once have been. As such, these "fixups" are in the cases I've
+seen in the logs, always incorrect.
+
+It therefore makes more sense to ignore the database data rather than
+corrupt the file permissions or worse. Looking at the pseudo logs
+in my heavily reused build directories, the number of these
+errors is staggering. This issue would explain many weird bugs we've
+seen over the years.
+
+There is a risk that we could not map permissions in some case where
+we currently would. I have not seen evidence of this in any logs I've
+read though. This change, whilst going against the original design,
+is in my view the safer option for the project at this point given we
+don't use pseudo as originally designed and never will be able to.
+
+Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
+Upstream-Status: Pending
+
+Index: git/pseudo.c
+===================================================================
+--- git.orig/pseudo.c
++++ git/pseudo.c
+@@ -699,6 +701,7 @@ pseudo_op(pseudo_msg_t *msg, const char
+ 						(unsigned long long) msg_header.ino,
+ 						path_by_ino ? path_by_ino : "no path",
+ 						msg->path);
++					found_ino = 0;
+ 				}
+ 			}
+ 		} else {
diff --git a/meta/recipes-devtools/pseudo/pseudo_git.bb b/meta/recipes-devtools/pseudo/pseudo_git.bb
index 3b623d8bd77..7eb72f0eab3 100644
--- a/meta/recipes-devtools/pseudo/pseudo_git.bb
+++ b/meta/recipes-devtools/pseudo/pseudo_git.bb
@@ -2,6 +2,7 @@ require pseudo.inc
 
 SRC_URI = "git://git.yoctoproject.org/pseudo;branch=oe-core \
            file://0001-configure-Prune-PIE-flags.patch \
+           file://delete_mismatches.patch \
            file://fallback-passwd \
            file://fallback-group \
            "
-- 
2.25.1

Re: [OE-core] [PATCH 2/4] pseudo: Ignore mismatched inodes from the db

[Seebs]

On Mon, 28 Sep 2020 14:38:01 +0100
"Richard Purdie" <richard.purdie@linuxfoundation.org> 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

Re: [OE-core] [PATCH 2/4] pseudo: Ignore mismatched inodes from the db

[Richard Purdie]

On Mon, 2020-09-28 at 09:13 -0500, Seebs wrote:
> On Mon, 28 Sep 2020 14:38:01 +0100
> "Richard Purdie" <richard.purdie@linuxfoundation.org> 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.

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.

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

I agree, which is why I spent the weekend making patch 3/4 work.

The big win of the next patch is that it reduces the entries in the
database to only the ones we actually care about. This doesn't tell
pseudo about specific files but of specific directories of files. The
result as mentioned in the patch is two orders of magnitude fewer files
in some sample pseudo database and the files left are the ones we care
about. Its not perfect, there are still a few that shouldn't be there
but it is an improvement.

I can't see any other way to handle this for the reasons I mention in
the patches, the performance hit for tracking all files would be too
great (and just moved the race window in many cases).

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

This patch tweaks one very specific corner case in a very specific way
since I've seen first hand that the current behaviour causes
corruption, I believe the alternative is safer.

I'm actually making pseudo do less magic fixups rather than more!

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.

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.

Cheers,

Richard

Re: [OE-core] [PATCH 2/4] pseudo: Ignore mismatched inodes from the db

[Seebs]

On Mon, 28 Sep 2020 15:51:53 +0100
"Richard Purdie" <richard.purdie@linuxfoundation.org> 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

Re: [OE-core] [PATCH 2/4] pseudo: Ignore mismatched inodes from the db

[Richard Purdie]

On Mon, 2020-09-28 at 10:56 -0500, Seebs wrote:
> On Mon, 28 Sep 2020 15:51:53 +0100
> "Richard Purdie" <richard.purdie@linuxfoundation.org> 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*.

Ultimately I agree.

Lets plan that we need to put an abort() here. We're not quite at the
point we can do that and I think changing the behaviour is a reasonable
first step so I do plan to add this patch however I will plan some
further patches to turn it into an abort(). How quickly I can do that
will depend upon what kind of issues it throws up.

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

It helps to know that! I wasn't sure if you'd hate the path filtering.

Cheers,

Richard

Re: [OE-core] [PATCH 2/4] pseudo: Ignore mismatched inodes from the db

[Seebs]

On Mon, 28 Sep 2020 22:42:18 +0100
"Richard Purdie" <richard.purdie@linuxfoundation.org> 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

[PATCH 3/4] pseudo: Add support for ignoring paths from the pseudo DB

[Richard Purdie]

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 .../pseudo/files/add_ignore_paths.patch       | 294 ++++++++++++++++++
 meta/recipes-devtools/pseudo/pseudo_git.bb    |   1 +
 2 files changed, 295 insertions(+)
 create mode 100644 meta/recipes-devtools/pseudo/files/add_ignore_paths.patch

diff --git a/meta/recipes-devtools/pseudo/files/add_ignore_paths.patch b/meta/recipes-devtools/pseudo/files/add_ignore_paths.patch
new file mode 100644
index 00000000000..eda275cf77c
--- /dev/null
+++ b/meta/recipes-devtools/pseudo/files/add_ignore_paths.patch
@@ -0,0 +1,294 @@
+Currently, pseudo considers any path accessed whist its running to be
+a valid entry to track in its database. The way OpenEmbedded uses pseudo,
+there are paths we care about accesses to from a pseudo perspective and paths
+which we simply don't care about.
+
+This patch adds a PSEUDO_IGNORE_PATHS environment variable which is a comma
+separated list of path prefixes to ignore accesses to.
+
+To do this, we add some functions which can check a path argument or a file
+descriptor argument and use these in the pseudo wrappers where path or fd
+arguments are present. Where paths are being ignored, we skip straight to
+the underlying real function.
+
+Psuedo needs to keep track of the open fd mappings to files so we still need
+to allow those cases into the pseudo_op function. Specficially this means
+OP_CLOSE, OP_OPEN, OP_DUP and OP_CHDIR.
+
+Apart from OP_OPEN which could call the server, the other operations are client
+side only so passed through. We 'tag' the functions using these operations so
+that the path ignore code isn't triggered. For OP_OPEN we exit early and skip
+the server op. We also have a catch all in client_op to ensure any operatings
+we didn't manage to skip early still get skipped correctly.
+
+OP_CHROOT is a special case. Where ignored path prefixes are used as a chroot,
+for the lifetime of the chroot, the path is effectively dropped from the
+PSEUDO_IGNORE_PATHS list. Whilst slightly counter intuaitive, this turned out
+to be the most effective way to do things due to commands like useradd and
+their use of chroots.
+
+For sqlite3 and appropriate path filtering in OE, this took the database from
+45,000 entries to about 180. For dbus this was 88,000 down to 760. Given the
+number of client to server trips these numbers of paths involves, the win
+is seemingly worthwhile.
+
+Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
+Upstream-Status: Pending
+
+Index: git/pseudo_client.c
+===================================================================
+--- git.orig/pseudo_client.c
++++ git/pseudo_client.c
+@@ -1482,6 +1482,39 @@ base_path(int dirfd, const char *path, i
+ 	return newpath;
+ }
+ 
++int pseudo_client_ignore_fd(int fd) {
++	if (fd >= 0 && fd <= nfds)
++		return pseudo_client_ignore_path(fd_path(fd));
++	return 0;
++}
++
++int pseudo_client_ignore_path(const char *path) {
++	char *env;
++	if (path) {
++		if (pseudo_chroot && strncmp(path, pseudo_chroot, pseudo_chroot_len) == 0)
++			return 0;
++		env = pseudo_get_value("PSEUDO_IGNORE_PATHS");
++		if (env) {
++			char *p = env;
++        	        while (*p) {
++				char *next = strchr(p, ',');
++				if (!next)
++				    next = strchr(p, '\0');
++				if (strncmp(path, p, next - p) == 0) {
++		 			pseudo_debug(PDBGF_PATH | PDBGF_VERBOSE, "ignoring path: '%s'\n", path);
++					return 1;
++				}
++				if (next && *next != '\0')
++					p = next+1;
++				else
++					break;
++			}
++			free(env);
++		}
++	}
++	return 0;
++}
++
+ pseudo_msg_t *
+ pseudo_client_op(pseudo_op_t op, int access, int fd, int dirfd, const char *path, const PSEUDO_STATBUF *buf, ...) {
+ 	pseudo_msg_t *result = 0;
+@@ -1522,6 +1555,16 @@ pseudo_client_op(pseudo_op_t op, int acc
+ 		}
+ 	}
+ 
++	if (op != OP_CHROOT && op != OP_CHDIR && op != OP_CLOSE && op != OP_DUP
++			&& pseudo_client_ignore_path(path)) {
++		if (op == OP_OPEN) {
++			pseudo_client_path(fd, path);
++		}
++		/* reenable wrappers */
++		pseudo_magic();
++		return result;
++	}
++
+ #ifdef PSEUDO_XATTRDB
+ 	if (buf) {
+ 		struct stat64 bufcopy = *buf;
+Index: git/pseudo_util.c
+===================================================================
+--- git.orig/pseudo_util.c
++++ git/pseudo_util.c
+@@ -43,6 +43,7 @@ static struct pseudo_variables pseudo_en
+ 	{ "PSEUDO_BINDIR", 13, NULL },
+ 	{ "PSEUDO_LIBDIR", 13, NULL },
+ 	{ "PSEUDO_LOCALSTATEDIR", 20, NULL },
++	{ "PSEUDO_IGNORE_PATHS", 19, NULL },
+ 	{ "PSEUDO_PASSWD", 13, NULL },
+ 	{ "PSEUDO_CHROOT", 13, NULL },
+ 	{ "PSEUDO_UIDS", 11, NULL },
+Index: git/pseudo_client.h
+===================================================================
+--- git.orig/pseudo_client.h
++++ git/pseudo_client.h
+@@ -7,6 +7,8 @@
+  *
+  */
+ extern pseudo_msg_t *pseudo_client_op(pseudo_op_t op, int access, int fd, int dirfd, const char *path, const PSEUDO_STATBUF *buf, ...);
++extern int pseudo_client_ignore_path(const char *path);
++extern int pseudo_client_ignore_fd(int fd);
+ #if PSEUDO_STATBUF_64
+ #define base_lstat real_lstat64
+ #define base_fstat real_fstat64
+Index: git/ports/linux/wrapfuncs.in
+===================================================================
+--- git.orig/ports/linux/wrapfuncs.in
++++ git/ports/linux/wrapfuncs.in
+@@ -1,23 +1,23 @@
+-int open(const char *path, int flags, ...{mode_t mode}); /* flags=flags&O_NOFOLLOW */
++int open(const char *path, int flags, ...{mode_t mode}); /* flags=flags&O_NOFOLLOW, noignore_path=1 */
+ char *get_current_dir_name(void);
+ int __xstat(int ver, const char *path, struct stat *buf);
+ int __lxstat(int ver, const char *path, struct stat *buf); /* flags=AT_SYMLINK_NOFOLLOW */
+ int __fxstat(int ver, int fd, struct stat *buf);
+ int lchown(const char *path, uid_t owner, gid_t group); /* flags=AT_SYMLINK_NOFOLLOW */
+ int __fxstatat(int ver, int dirfd, const char *path, struct stat *buf, int flags);
+-int openat(int dirfd, const char *path, int flags, ...{mode_t mode}); /* flags=flags&O_NOFOLLOW */
+-int __openat_2(int dirfd, const char *path, int flags); /* flags=flags&O_NOFOLLOW */
++int openat(int dirfd, const char *path, int flags, ...{mode_t mode}); /* flags=flags&O_NOFOLLOW, noignore_path=1 */
++int __openat_2(int dirfd, const char *path, int flags); /* flags=flags&O_NOFOLLOW, noignore_path=1 */
+ int mknod(const char *path, mode_t mode, dev_t dev); /* real_func=pseudo_mknod */
+ int mknodat(int dirfd, const char *path, mode_t mode, dev_t dev); /* real_func=pseudo_mknodat */
+ int __xmknod(int ver, const char *path, mode_t mode, dev_t *dev); /* flags=AT_SYMLINK_NOFOLLOW */
+ int __xmknodat(int ver, int dirfd, const char *path, mode_t mode, dev_t *dev); /* flags=AT_SYMLINK_NOFOLLOW */
+-int fcntl(int fd, int cmd, ...{struct flock *lock});
++int fcntl(int fd, int cmd, ...{struct flock *lock});  /* noignore_path=1 */
+ # just so we know the inums of symlinks
+ char *canonicalize_file_name(const char *filename);
+ int eaccess(const char *path, int mode);
+-int open64(const char *path, int flags, ...{mode_t mode}); /* flags=flags&O_NOFOLLOW */
+-int openat64(int dirfd, const char *path, int flags, ...{mode_t mode}); /* flags=flags&O_NOFOLLOW */
+-int __openat64_2(int dirfd, const char *path, int flags); /* flags=flags&O_NOFOLLOW */
++int open64(const char *path, int flags, ...{mode_t mode}); /* flags=flags&O_NOFOLLOW, noignore_path=1 */
++int openat64(int dirfd, const char *path, int flags, ...{mode_t mode}); /* flags=flags&O_NOFOLLOW, noignore_path=1 */
++int __openat64_2(int dirfd, const char *path, int flags); /* flags=flags&O_NOFOLLOW, noignore_path=1 */
+ int creat64(const char *path, mode_t mode);
+ int stat(const char *path, struct stat *buf); /* real_func=pseudo_stat */
+ int lstat(const char *path, struct stat *buf); /* real_func=pseudo_lstat, flags=AT_SYMLINK_NOFOLLOW */
+@@ -29,9 +29,9 @@ int __xstat64(int ver, const char *path,
+ int __lxstat64(int ver, const char *path, struct stat64 *buf); /* flags=AT_SYMLINK_NOFOLLOW */
+ int __fxstat64(int ver, int fd, struct stat64 *buf);
+ int __fxstatat64(int ver, int dirfd, const char *path, struct stat64 *buf, int flags);
+-FILE *fopen64(const char *path, const char *mode);
+-int nftw64(const char *path, int (*fn)(const char *, const struct stat64 *, int, struct FTW *), int nopenfd, int flag);
+-FILE *freopen64(const char *path, const char *mode, FILE *stream);
++FILE *fopen64(const char *path, const char *mode); /* noignore_path=1 */
++int nftw64(const char *path, int (*fn)(const char *, const struct stat64 *, int, struct FTW *), int nopenfd, int flag); /* noignore_path=1 */
++FILE *freopen64(const char *path, const char *mode, FILE *stream);  /* noignore_path=1 */
+ int ftw64(const char *path, int (*fn)(const char *, const struct stat64 *, int), int nopenfd);
+ int glob64(const char *pattern, int flags, int (*errfunc)(const char *, int), glob64_t *pglob);
+ int scandir64(const char *path, struct dirent64 ***namelist, int (*filter)(const struct dirent64 *), int (*compar)());
+Index: git/templates/wrapfuncs.c
+===================================================================
+--- git.orig/templates/wrapfuncs.c
++++ git/templates/wrapfuncs.c
+@@ -60,9 +60,15 @@ ${maybe_async_skip}
+ 		${rc_assign} (*real_${name})(${call_args});
+ 	} else {
+ 		${fix_paths}
+-		/* exec*() use this to restore the sig mask */
+-		pseudo_saved_sigmask = saved;
+-		${rc_assign} wrap_$name(${call_args});
++		if (${ignore_paths}) {
++			/* call the real syscall */
++			pseudo_debug(PDBGF_SYSCALL, "${name} ignored path, calling real syscall.\n");
++			${rc_assign} (*real_${name})(${call_args});
++		} else {
++			/* exec*() use this to restore the sig mask */
++			pseudo_saved_sigmask = saved;
++			${rc_assign} wrap_$name(${call_args});
++		}
+ 	}
+ 	${variadic_end}
+ 	save_errno = errno;
+Index: git/ports/unix/wrapfuncs.in
+===================================================================
+--- git.orig/ports/unix/wrapfuncs.in
++++ git/ports/unix/wrapfuncs.in
+@@ -1,14 +1,14 @@
+ int creat(const char *path, mode_t mode);
+ char *getcwd(char *buf, size_t size);
+ char *getwd(char *buf);
+-int close(int fd);
++int close(int fd);  /* noignore_path=1 */
+ int fchmod(int fd, mode_t mode);
+ int fchown(int fd, uid_t owner, gid_t group);
+ int lchown(const char *path, uid_t owner, gid_t group); /* flags=AT_SYMLINK_NOFOLLOW */
+-int dup2(int oldfd, int newfd);
+-int dup(int fd);
+-int chdir(const char *path);
+-int fchdir(int dirfd);
++int dup2(int oldfd, int newfd); /* noignore_path=1 */
++int dup(int fd); /* noignore_path=1 */
++int chdir(const char *path); /* noignore_path=1 */
++int fchdir(int dirfd); /* noignore_path=1 */
+ int access(const char *path, int mode);
+ FTS *fts_open(char * const *path_argv, int options, int (*compar)(const FTSENT **, const FTSENT **)); /* inode64=1 */
+ int ftw(const char *path, int (*fn)(const char *, const struct stat *, int), int nopenfd);
+@@ -20,18 +20,18 @@ char *mktemp(char *template);
+ long pathconf(const char *path, int name);
+ char *realpath(const char *name, char *resolved_name); /* version="GLIBC_2.3" */
+ int remove(const char *path); /* flags=AT_SYMLINK_NOFOLLOW */
+-DIR *opendir(const char *path);
+-int closedir(DIR *dirp);
++DIR *opendir(const char *path); /* noignore_path=1 */
++int closedir(DIR *dirp); /* noignore_path=1 */
+ char *tempnam(const char *template, const char *pfx);
+ char *tmpnam(char *s);
+ int truncate(const char *path, off_t length);
+ int utime(const char *path, const struct utimbuf *buf);
+ int utimes(const char *path, const struct timeval *times);
+ # needed because libc stdio does horrible things with inline asm syscalls
+-FILE *fopen(const char *path, const char *mode);
+-int fclose(FILE *fp);
+-FILE *freopen(const char *path, const char *mode, FILE *stream);
+-int chroot(const char *path);
++FILE *fopen(const char *path, const char *mode); /* noignore_path=1 */
++int fclose(FILE *fp);  /* noignore_path=1 */
++FILE *freopen(const char *path, const char *mode, FILE *stream); /* noignore_path=1 */
++int chroot(const char *path); /* noignore_path=1 */
+ int acct(const char *path);
+ int chmod(const char *path, mode_t mode);
+ int chown(const char *path, uid_t owner, gid_t group);
+Index: git/makewrappers
+===================================================================
+--- git.orig/makewrappers
++++ git/makewrappers
+@@ -228,6 +230,8 @@ class Function:
+         self.real_func = None
+         self.paths_to_munge = []
+         self.specific_dirfds = {}
++        self.fd_arg = False
++        self.noignore_path = False
+         self.hand_wrapped = None
+         self.async_skip = None
+         # used for the copyright date when creating stub functions
+@@ -267,6 +271,11 @@ class Function:
+                 self.flags = '(flags & AT_SYMLINK_NOFOLLOW)'
+             elif arg.name.endswith('path'):
+                 self.paths_to_munge.append(arg.name)
++            elif arg.name == 'fd':
++                self.fd_arg = "fd"
++            elif arg.name == 'filedes':
++                self.fd_arg = "filedes"
++
+     
+         # pick default values
+         if self.type == 'void':
+@@ -361,6 +370,25 @@ class Function:
+                 (path, prefix, self.dirfd, path, self.flags))
+         return "\n\t\t".join(fix_paths)
+ 
++    def ignore_paths(self):
++        if self.noignore_path:
++            return "0"
++
++        mainpath = None
++        if "oldpath" in self.paths_to_munge:
++            mainpath = "oldpath"
++        elif "newpath" in self.paths_to_munge:
++            mainpath = "newpath"
++        elif "path" in self.paths_to_munge:
++            mainpath = "path"
++
++        if mainpath:
++            return "pseudo_client_ignore_path(%s)" % mainpath
++        if self.fd_arg:
++            return "pseudo_client_ignore_fd(%s)" % self.fd_arg
++
++        return "0"
++
+     def real_predecl(self):
+         if self.real_func:
+             return self.decl().replace(self.name, self.real_func, 1) + ";"
diff --git a/meta/recipes-devtools/pseudo/pseudo_git.bb b/meta/recipes-devtools/pseudo/pseudo_git.bb
index 7eb72f0eab3..57c32d0cfc1 100644
--- a/meta/recipes-devtools/pseudo/pseudo_git.bb
+++ b/meta/recipes-devtools/pseudo/pseudo_git.bb
@@ -3,6 +3,7 @@ require pseudo.inc
 SRC_URI = "git://git.yoctoproject.org/pseudo;branch=oe-core \
            file://0001-configure-Prune-PIE-flags.patch \
            file://delete_mismatches.patch \
+           file://add_ignore_paths.patch \
            file://fallback-passwd \
            file://fallback-group \
            "
-- 
2.25.1

[PATCH 4/4] base/bitbake.conf: Enable pseudo path filtering

[Richard Purdie]

This is a pretty big change to the way pseudo operates when used in OpenEmbedded.
Normally, pseudo monitors and logs (adds to its database) any file created or
modified whilst in a fakeroot environment. There are large numbers of files
we simply don't care about the permissions of whilst in that fakeroot context,
for example ${S}, ${B}, ${T}, ${SSTATE_DIR}, the central sstate control
directories,

This change uses new functionality in pseudo to ignore these directory trees,
resulting in a cleaner database with less chance of "stray" mismatches if files
are modified outside pseudo context. It also should reduce some overhead from
pseudo as the interprocess round trip to the server is avoided.

There is a possible complication where some existing recipe may break, for
example, we found a recipe which was writing to "${B}/install" for
"make install" in do_install and since we listed ${B} as not to be tracked,
there were errors trying to chown root for files in this location.

This patch fixes a few corner cases in OE-Core when used with this new
ignore list:

* The archiver directory matched a "${WORKDIR}/deploy*" pattern so was renamed
  to something else since that directory does need its root permissions
* The ${S} and ${B} ignoring is conditional on them being different to ${WORKDIR}
* package_write_* task output (the debs/rpms/ipks) are now owned by the build
  user so we don't want the file ownership information in the hashequiv outhash
  calculation even if they are built under pseudo.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 meta/classes/archiver.bbclass          | 2 +-
 meta/classes/base.bbclass              | 5 +++++
 meta/classes/populate_sdk_base.bbclass | 2 ++
 meta/conf/bitbake.conf                 | 1 +
 meta/lib/oe/sstatesig.py               | 2 ++
 5 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/meta/classes/archiver.bbclass b/meta/classes/archiver.bbclass
index aff1f9dbb04..598298ef6f6 100644
--- a/meta/classes/archiver.bbclass
+++ b/meta/classes/archiver.bbclass
@@ -53,7 +53,7 @@ ARCHIVER_MODE[recipe] ?= "0"
 ARCHIVER_MODE[mirror] ?= "split"
 
 DEPLOY_DIR_SRC ?= "${DEPLOY_DIR}/sources"
-ARCHIVER_TOPDIR ?= "${WORKDIR}/deploy-sources"
+ARCHIVER_TOPDIR ?= "${WORKDIR}/archiver-sources"
 ARCHIVER_OUTDIR = "${ARCHIVER_TOPDIR}/${TARGET_SYS}/${PF}/"
 ARCHIVER_RPMTOPDIR ?= "${WORKDIR}/deploy-sources-rpm"
 ARCHIVER_RPMOUTDIR = "${ARCHIVER_RPMTOPDIR}/${TARGET_SYS}/${PF}/"
diff --git a/meta/classes/base.bbclass b/meta/classes/base.bbclass
index 4c681cc870d..6309d709b8d 100644
--- a/meta/classes/base.bbclass
+++ b/meta/classes/base.bbclass
@@ -388,6 +388,11 @@ python () {
     oe.utils.features_backfill("DISTRO_FEATURES", d)
     oe.utils.features_backfill("MACHINE_FEATURES", d)
 
+    if d.getVar("WORKDIR") != d.getVar("S"):
+        d.appendVar("PSEUDO_IGNORE_PATHS", ",${S}")
+    if d.getVar("WORKDIR") != d.getVar("B"):
+        d.appendVar("PSEUDO_IGNORE_PATHS", ",${B}")
+
     # Handle PACKAGECONFIG
     #
     # These take the form:
diff --git a/meta/classes/populate_sdk_base.bbclass b/meta/classes/populate_sdk_base.bbclass
index 990505e89b8..61b31d5e5e5 100644
--- a/meta/classes/populate_sdk_base.bbclass
+++ b/meta/classes/populate_sdk_base.bbclass
@@ -178,6 +178,8 @@ do_populate_sdk[sstate-inputdirs] = "${SDKDEPLOYDIR}"
 do_populate_sdk[sstate-outputdirs] = "${SDK_DEPLOY}"
 do_populate_sdk[stamp-extra-info] = "${MACHINE_ARCH}${SDKMACHINE}"
 
+PSEUDO_IGNORE_PATHS .= ",${SDKDEPLOYDIR}"
+
 fakeroot create_sdk_files() {
 	cp ${COREBASE}/scripts/relocate_sdk.py ${SDK_OUTPUT}/${SDKPATH}/
 
diff --git a/meta/conf/bitbake.conf b/meta/conf/bitbake.conf
index a318d1ca588..ebc9c9b56b9 100644
--- a/meta/conf/bitbake.conf
+++ b/meta/conf/bitbake.conf
@@ -695,6 +695,7 @@ FAKEROOTENV = "PSEUDO_PREFIX=${PSEUDO_SYSROOT}${prefix_native} PSEUDO_LOCALSTATE
 FAKEROOTNOENV = "PSEUDO_UNLOAD=1"
 FAKEROOTDIRS = "${PSEUDO_LOCALSTATEDIR}"
 PREFERRED_PROVIDER_virtual/fakeroot-native ?= "pseudo-native"
+export PSEUDO_IGNORE_PATHS = "/usr/,/etc/,/lib,${T},${WORKDIR}/recipe-sysroot,${SSTATE_DIR},${STAMPS_DIR},${WORKDIR}/pkgdata-sysroot,${TMPDIR}/sstate-control,${DEPLOY_DIR},${WORKDIR}/deploy-,${TMPDIR}/buildstats"
 
 ##################################################################
 # Not sure about the rest of this yet.
diff --git a/meta/lib/oe/sstatesig.py b/meta/lib/oe/sstatesig.py
index 21ae0a7657f..6a8630048cf 100644
--- a/meta/lib/oe/sstatesig.py
+++ b/meta/lib/oe/sstatesig.py
@@ -482,6 +482,8 @@ def OEOuthashBasic(path, sigfile, task, d):
     h = hashlib.sha256()
     prev_dir = os.getcwd()
     include_owners = os.environ.get('PSEUDO_DISABLED') == '0'
+    if "package_write_" in task:
+        include_owners = False
     extra_content = d.getVar('HASHEQUIV_HASH_VERSION')
 
     try:
-- 
2.25.1

Re: [OE-core] [PATCH 1/4] staging: Ensure cleaned dependencies are added

[Christopher Larson]

[-- Attachment #1: Type: text/plain, Size: 2003 bytes --]

I'd suggest using parenthesis to clarify the order here. 'a or b and c'
isn't intuitively clear unless you know the or vs and operator precedence.

On Mon, Sep 28, 2020 at 6:38 AM Richard Purdie <
richard.purdie@linuxfoundation.org> wrote:

> Most recipe-sysroot dependencies are handled by these prefuncs. "configure"
> is special with a decidated task, prepare_recipe_sysroot which runs
> beforehand.
>
> do_prepare_recipe_sysroot does not have to be run before/after
> fetch/unpack/patch, they're independent tasks. If fetch/unpack/patch
> have sysroot dependencies and those tasks rerun, stale items from the
> sysroot could be uninstalled and since prepare_recipe_sysroot doesn't
> re-run, they could be missing by the time configure runs.
>
> Fix this by adding the prefunc for do_configure which would ensure
> the sysroot is in a good state.
>
> This issue can be reproduced by modifying do_unpack and watching
> binutils-native and vulkan-headers failing due to missing makeinfo
> or ninja.
>
> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> ---
>  meta/classes/staging.bbclass | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/meta/classes/staging.bbclass b/meta/classes/staging.bbclass
> index de3a19815a3..7c9fb38607e 100644
> --- a/meta/classes/staging.bbclass
> +++ b/meta/classes/staging.bbclass
> @@ -614,7 +614,7 @@ python staging_taskhandler() {
>      bbtasks = e.tasklist
>      for task in bbtasks:
>          deps = d.getVarFlag(task, "depends")
> -        if deps and "populate_sysroot" in deps:
> +        if task == "do_configure" or deps and "populate_sysroot" in deps:
>              d.appendVarFlag(task, "prefuncs", " extend_recipe_sysroot")
>  }
>  staging_taskhandler[eventmask] = "bb.event.RecipeTaskPreProcess"
> --
> 2.25.1
>
>
> 
>
>

-- 
Christopher Larson
kergoth at gmail dot com
Founder - BitBake, OpenEmbedded, OpenZaurus
Senior Software Engineer, Mentor Graphics

[-- Attachment #2: Type: text/html, Size: 2723 bytes --]

Re: [OE-core] [PATCH 1/4] staging: Ensure cleaned dependencies are added

[Richard Purdie]

On Mon, 2020-09-28 at 08:15 -0700, Christopher Larson wrote:
> I'd suggest using parenthesis to clarify the order here. 'a or b and
> c' isn't intuitively clear unless you know the or vs and operator
> precedence.

Good point, I've tweaked that in master-next.

Cheers,

Richard

Re: [OE-core] [PATCH 1/4] staging: Ensure cleaned dependencies are added

[Chris Laplante]

Hi Richard,

> Most recipe-sysroot dependencies are handled by these prefuncs. "configure"
> is special with a decidated task, prepare_recipe_sysroot which runs
> beforehand.
> 
> do_prepare_recipe_sysroot does not have to be run before/after
> fetch/unpack/patch, they're independent tasks. If fetch/unpack/patch have
> sysroot dependencies and those tasks rerun, stale items from the sysroot
> could be uninstalled and since prepare_recipe_sysroot doesn't re-run, they
> could be missing by the time configure runs.
> 
> Fix this by adding the prefunc for do_configure which would ensure the
> sysroot is in a good state.

Is this patch something that should be backported to zeus and friends?

Chris

Re: [OE-core] [PATCH 1/4] staging: Ensure cleaned dependencies are added

[Richard Purdie]

On Tue, 2020-10-06 at 16:07 +0000, chris.laplante@agilent.com wrote:
> Hi Richard,
> 
> > Most recipe-sysroot dependencies are handled by these prefuncs.
> > "configure"
> > is special with a decidated task, prepare_recipe_sysroot which runs
> > beforehand.
> > 
> > do_prepare_recipe_sysroot does not have to be run before/after
> > fetch/unpack/patch, they're independent tasks. If
> > fetch/unpack/patch have
> > sysroot dependencies and those tasks rerun, stale items from the
> > sysroot
> > could be uninstalled and since prepare_recipe_sysroot doesn't re-
> > run, they
> > could be missing by the time configure runs.
> > 
> > Fix this by adding the prefunc for do_configure which would ensure
> > the
> > sysroot is in a good state.
> 
> Is this patch something that should be backported to zeus and
> friends?

Its fixing a very rare problem that I only ran into whilst doing
"interesting" things most people don't probably do.

If people were running into those issues I'd backport but for master it
was more a correctness fix which did fix some weird corner cases I
debugged...

Cheers,

Richard

Re: [OE-core] [PATCH 1/4] staging: Ensure cleaned dependencies are added

[Chris Laplante]

> > Is this patch something that should be backported to zeus and friends?
> 
> Its fixing a very rare problem that I only ran into whilst doing "interesting"
> things most people don't probably do.
> 
> If people were running into those issues I'd backport but for master it was
> more a correctness fix which did fix some weird corner cases I debugged...


A colleague today did see something that sounded to me like it may be fixed by this patch. It was a native tool that should have been available during a do_configure that wasn't. We didn't take the time to dig into it though - it was resolved with a -c cleansstate of the -native recipe and the target recipe. 

Chris

Re: [OE-core] [PATCH 1/4] staging: Ensure cleaned dependencies are added

[Richard Purdie]

On Tue, 2020-10-06 at 16:59 +0000, chris.laplante@agilent.com wrote:
> > > Is this patch something that should be backported to zeus and
> > > friends?
> > 
> > Its fixing a very rare problem that I only ran into whilst doing
> > "interesting"
> > things most people don't probably do.
> > 
> > If people were running into those issues I'd backport but for
> > master it was
> > more a correctness fix which did fix some weird corner cases I
> > debugged...
> 
> A colleague today did see something that sounded to me like it may be
> fixed by this patch. It was a native tool that should have been
> available during a do_configure that wasn't. We didn't take the time
> to dig into it though - it was resolved with a -c cleansstate of the
> -native recipe and the target recipe. 

I kind of wish people would dig into these things as its the only real
way we'll get them fixed. If it was this issue, removing the
prepare_recipe_sysroot stamp should have sorted the problem.

The issue is there in zeus so I guess we could need this fix there.
Question is whether its worth any other inadvertent breakage.

Cheers,

Richard

Re: [OE-core] [PATCH 1/4] staging: Ensure cleaned dependencies are added

[Chris Laplante]

> > A colleague today did see something that sounded to me like it may be
> > fixed by this patch. It was a native tool that should have been
> > available during a do_configure that wasn't. We didn't take the time
> > to dig into it though - it was resolved with a -c cleansstate of the
> > -native recipe and the target recipe.
> 
> I kind of wish people would dig into these things as its the only real way we'll
> get them fixed. If it was this issue, removing the prepare_recipe_sysroot stamp
> should have sorted the problem.
> 
> The issue is there in zeus so I guess we could need this fix there.
> Question is whether its worth any other inadvertent breakage.


I'll dig in the next time it happens and report back.

Thanks,
Chris