From b5c5d84b8be27c611c1c21d5c13039a90977bbd4 Mon Sep 17 00:00:00 2001 From: Richard Purdie Date: Tue, 22 Sep 2020 14:07:48 +0100 Subject: pseudo: Ignore mismatched inodes from the db 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. (From OE-Core rev: 1c13149b81e03a1ac48b27a208a139d5493c3ce7) Signed-off-by: Richard Purdie --- .../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 (limited to 'meta/recipes-devtools') 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 0000000000..6c78d787c7 --- /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 +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 3b623d8bd7..7eb72f0eab 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 \ " -- cgit v1.2.3-54-g00ecf