* [PATCH 1/2] pseudo: do not expand symlinks in /proc @ 2020-10-07 16:20 Richard Purdie 2020-10-07 16:20 ` [PATCH 2/2] pseudo: Fix statx function usage Richard Purdie 0 siblings, 1 reply; 6+ messages in thread From: Richard Purdie @ 2020-10-07 16:20 UTC (permalink / raw) To: openembedded-core; +Cc: seebs Some symlinks in /proc, such as those under /proc/[pid]/fd, /proc/[pid]/cwd, and /proc/[pid]/exe that are not real and should not have readlink called on them. These look like symlinks, but behave like hardlinks. Readlink does not return actual paths. Previously pseudo_fix_path would expand files such as /dev/stdin to paths such as /proc/6680/fd/pipe:[1270830076] which do not exist. This issue affects: - deleted files - deleted directories - fifos - sockets - anon_inodes (epoll, eventfd, inotify, signalfd, timerfd, etc) Testing: timed builds before and after applying patch, without significant measurable difference. $ bitbake -c compile <image>; time bitbake <image> installed pseudo on an image and was unable to reproduce the test on bug report after applying the patch. [YOCTO #13288] Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org> --- meta/recipes-devtools/pseudo/files/proc.patch | 86 +++++++++++++++++++ meta/recipes-devtools/pseudo/pseudo_git.bb | 1 + 2 files changed, 87 insertions(+) create mode 100644 meta/recipes-devtools/pseudo/files/proc.patch diff --git a/meta/recipes-devtools/pseudo/files/proc.patch b/meta/recipes-devtools/pseudo/files/proc.patch new file mode 100644 index 00000000000..57ae9bf9504 --- /dev/null +++ b/meta/recipes-devtools/pseudo/files/proc.patch @@ -0,0 +1,86 @@ +From: Matt Cowell <matt.cowell@nokia.com> +From: "Sakib Sajal" <sakib.sajal@windriver.com> + +pseudo: do not expand symlinks in /proc + +Some symlinks in /proc, such as those under /proc/[pid]/fd, +/proc/[pid]/cwd, and /proc/[pid]/exe that are not real and should not +have readlink called on them. These look like symlinks, but behave like +hardlinks. Readlink does not return actual paths. Previously +pseudo_fix_path would expand files such as /dev/stdin to paths such as +/proc/6680/fd/pipe:[1270830076] which do not exist. + +This issue affects: +- deleted files +- deleted directories +- fifos +- sockets +- anon_inodes (epoll, eventfd, inotify, signalfd, timerfd, etc) + +Testing: +timed builds before and after applying patch, without significant +measurable difference. +$ bitbake -c compile <image>; time bitbake <image> + +installed pseudo on an image and was unable to reproduce the test +on bug report after applying the patch. + +[YOCTO #13288] + +Signed-off-by: Sakib Sajal <sakib.sajal@windriver.com> +--- + pseudo_util.c | 27 +++++++++++++++++++++++++++ + 1 file changed, 27 insertions(+) + +diff --git a/pseudo_util.c b/pseudo_util.c +index c867ed6..bce4d1e 100644 +--- a/pseudo_util.c ++++ b/pseudo_util.c +@@ -21,6 +21,8 @@ + #include <sys/time.h> + #include <unistd.h> + #include <limits.h> ++#include <sys/vfs.h> ++#include <linux/magic.h> + + /* see the comments below about (*real_regcomp)() */ + #include <dlfcn.h> +@@ -29,6 +31,11 @@ + #include "pseudo_ipc.h" + #include "pseudo_db.h" + ++/* O_PATH is defined in glibc 2.16 and later only */ ++#ifndef O_PATH ++#define O_PATH 010000000 ++#endif ++ + struct pseudo_variables { + char *key; + size_t key_len; +@@ -677,6 +684,26 @@ pseudo_append_element(char *newpath, char *root, size_t allocated, char **pcurre + */ + if (!leave_this && is_dir) { + int is_link = S_ISLNK(buf->st_mode); ++ ++ /* do not expand symlinks in the proc filesystem, since they may not be real */ ++ if (is_link) { ++ struct statfs sfs; ++ int fd; ++ ++ /* statfs follows symlinks, so use fstatfs */ ++ fd = open(newpath, O_CLOEXEC | O_PATH | O_NOFOLLOW); ++ if (-1 != fd) { ++ if (0 == fstatfs(fd, &sfs) && sfs.f_type == PROC_SUPER_MAGIC) { ++ pseudo_debug(PDBGF_PATH | PDBGF_VERBOSE, ++ "pae: '%s' is procfs symlink, not expanding\n", ++ newpath); ++ is_link = 0; ++ } ++ ++ close(fd); ++ } ++ } ++ + if (link_recursion >= PSEUDO_MAX_LINK_RECURSION && is_link) { + pseudo_diag("link recursion too deep, not expanding path '%s'.\n", newpath); + is_link = 0; diff --git a/meta/recipes-devtools/pseudo/pseudo_git.bb b/meta/recipes-devtools/pseudo/pseudo_git.bb index bc20a2f134e..90f5cb0bcf2 100644 --- a/meta/recipes-devtools/pseudo/pseudo_git.bb +++ b/meta/recipes-devtools/pseudo/pseudo_git.bb @@ -9,6 +9,7 @@ SRC_URI = "git://git.yoctoproject.org/pseudo;branch=oe-core \ file://xattr_fix.patch \ file://mayunlink.patch \ file://pathfix.patch \ + file://proc.patch \ file://fallback-passwd \ file://fallback-group \ " -- 2.25.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] pseudo: Fix statx function usage 2020-10-07 16:20 [PATCH 1/2] pseudo: do not expand symlinks in /proc Richard Purdie @ 2020-10-07 16:20 ` Richard Purdie 2020-10-12 19:08 ` [OE-core] " Seebs 0 siblings, 1 reply; 6+ messages in thread From: Richard Purdie @ 2020-10-07 16:20 UTC (permalink / raw) To: openembedded-core; +Cc: seebs There is magic in the posts where specific variable names have specific magic. For that magic to work, "path" needs to be used not "pathname" as is currently there. Fix this, which fixes path issues on systems using statx (Ubuntu 20.04 in particular). Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org> --- .../pseudo/files/statxfix.patch | 56 +++++++++++++++++++ meta/recipes-devtools/pseudo/pseudo_git.bb | 1 + 2 files changed, 57 insertions(+) create mode 100644 meta/recipes-devtools/pseudo/files/statxfix.patch diff --git a/meta/recipes-devtools/pseudo/files/statxfix.patch b/meta/recipes-devtools/pseudo/files/statxfix.patch new file mode 100644 index 00000000000..c47ff27f9f4 --- /dev/null +++ b/meta/recipes-devtools/pseudo/files/statxfix.patch @@ -0,0 +1,56 @@ +There is magic in the posts where specific variable names have specific +magic. For that magic to work, "path" needs to be used not "pathname" as +is currently there. Fix this, which fixes path issues on systems using +statx (Ubuntu 20.04 in particular). + +Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org> +Upstream-Status: Pending + +Index: git/ports/linux/statx/guts/statx.c +=================================================================== +--- git.orig/ports/linux/statx/guts/statx.c ++++ git/ports/linux/statx/guts/statx.c +@@ -5,14 +5,14 @@ + * SPDX-License-Identifier: LGPL-2.1-only + * + * int +- * statx(int dirfd, const char *pathname, int flags, unsigned int mask, struct statx *statxbuf) { ++ * statx(int dirfd, const char *path, int flags, unsigned int mask, struct statx *statxbuf) { + * int rc = -1; + */ + pseudo_msg_t *msg; + PSEUDO_STATBUF buf; + int save_errno; + +- rc = real_statx(dirfd, pathname, flags, mask, statxbuf); ++ rc = real_statx(dirfd, path, flags, mask, statxbuf); + save_errno = errno; + if (rc == -1) { + return rc; +@@ -25,16 +25,16 @@ + buf.st_mode = statxbuf->stx_mode; + buf.st_rdev = makedev(statxbuf->stx_rdev_major, statxbuf->stx_rdev_minor); + buf.st_nlink = statxbuf->stx_nlink; +- msg = pseudo_client_op(OP_STAT, 0, -1, dirfd, pathname, &buf); ++ msg = pseudo_client_op(OP_STAT, 0, -1, dirfd, path, &buf); + if (msg && msg->result == RESULT_SUCCEED) { +- pseudo_debug(PDBGF_FILE, "statx(path %s), flags %o, stat rc %d, stat uid %o\n", pathname, flags, rc, statxbuf->stx_uid); ++ pseudo_debug(PDBGF_FILE, "statx(path %s), flags %o, stat rc %d, stat uid %o\n", path, flags, rc, statxbuf->stx_uid); + statxbuf->stx_uid = msg->uid; + statxbuf->stx_gid = msg->gid; + statxbuf->stx_mode = msg->mode; + statxbuf->stx_rdev_major = major(msg->rdev); + statxbuf->stx_rdev_minor = minor(msg->rdev); + } else { +- pseudo_debug(PDBGF_FILE, "statx(path %s) failed, flags %o, stat rc %d, stat uid %o\n", pathname, flags, rc, statxbuf->stx_uid); ++ pseudo_debug(PDBGF_FILE, "statx(path %s) failed, flags %o, stat rc %d, stat uid %o\n", path, flags, rc, statxbuf->stx_uid); + } + errno = save_errno; + /* return rc; +Index: git/ports/linux/statx/wrapfuncs.in +=================================================================== +--- git.orig/ports/linux/statx/wrapfuncs.in ++++ git/ports/linux/statx/wrapfuncs.in +@@ -1 +1 @@ +-int statx(int dirfd, const char *pathname, int flags, unsigned int mask, struct statx *statxbuf); ++int statx(int dirfd, const char *path, int flags, unsigned int mask, struct statx *statxbuf); diff --git a/meta/recipes-devtools/pseudo/pseudo_git.bb b/meta/recipes-devtools/pseudo/pseudo_git.bb index 90f5cb0bcf2..626e658f65d 100644 --- a/meta/recipes-devtools/pseudo/pseudo_git.bb +++ b/meta/recipes-devtools/pseudo/pseudo_git.bb @@ -10,6 +10,7 @@ SRC_URI = "git://git.yoctoproject.org/pseudo;branch=oe-core \ file://mayunlink.patch \ file://pathfix.patch \ file://proc.patch \ + file://statxfix.patch \ file://fallback-passwd \ file://fallback-group \ " -- 2.25.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [OE-core] [PATCH 2/2] pseudo: Fix statx function usage 2020-10-07 16:20 ` [PATCH 2/2] pseudo: Fix statx function usage Richard Purdie @ 2020-10-12 19:08 ` Seebs 2020-10-12 19:14 ` Mark Hatle 2020-10-12 20:25 ` Richard Purdie 0 siblings, 2 replies; 6+ messages in thread From: Seebs @ 2020-10-12 19:08 UTC (permalink / raw) To: Richard Purdie; +Cc: openembedded-core On Wed, 7 Oct 2020 17:20:18 +0100 "Richard Purdie" <richard.purdie@linuxfoundation.org> wrote: > +There is magic in the posts where specific variable names have > specific +magic. For that magic to work, "path" needs to be used not > "pathname" as +is currently there. Fix this, which fixes path issues > on systems using +statx (Ubuntu 20.04 in particular). So, usually if "pathname" is used, it's intentional to suppress the special magic behavior. Usually, but not always. In this case, the man page does use "pathname" and sometimes that means it's just a cut and paste error. My man page also says there's no glibc wrapper for statx, making me slightly surprised that a wrapper for it is needed/relevant. -s ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [OE-core] [PATCH 2/2] pseudo: Fix statx function usage 2020-10-12 19:08 ` [OE-core] " Seebs @ 2020-10-12 19:14 ` Mark Hatle 2020-10-12 19:21 ` Seebs 2020-10-12 20:25 ` Richard Purdie 1 sibling, 1 reply; 6+ messages in thread From: Mark Hatle @ 2020-10-12 19:14 UTC (permalink / raw) To: Seebs, Richard Purdie; +Cc: openembedded-core On 10/12/20 2:08 PM, Seebs wrote: > On Wed, 7 Oct 2020 17:20:18 +0100 > "Richard Purdie" <richard.purdie@linuxfoundation.org> wrote: > >> +There is magic in the posts where specific variable names have >> specific +magic. For that magic to work, "path" needs to be used not >> "pathname" as +is currently there. Fix this, which fixes path issues >> on systems using +statx (Ubuntu 20.04 in particular). > > So, usually if "pathname" is used, it's intentional to suppress the > special magic behavior. Usually, but not always. In this case, the man > page does use "pathname" and sometimes that means it's just a cut and > paste error. > > My man page also says there's no glibc wrapper for statx, making me > slightly surprised that a wrapper for it is needed/relevant. I looked into this. It appears (after pseudo was written, but) about 7 or so years ago, all of the man pages were re-written and the language was changed. So 'pathname' no longer has the same syntactic meaning that it did when pseudo was first written. (It's not just statx, I looked at things like fopen, open, etc.) --Mark > -s > > > > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [OE-core] [PATCH 2/2] pseudo: Fix statx function usage 2020-10-12 19:14 ` Mark Hatle @ 2020-10-12 19:21 ` Seebs 0 siblings, 0 replies; 6+ messages in thread From: Seebs @ 2020-10-12 19:21 UTC (permalink / raw) To: Mark Hatle; +Cc: Richard Purdie, openembedded-core On Mon, 12 Oct 2020 14:14:36 -0500 "Mark Hatle" <mark.hatle@kernel.crashing.org> wrote: > I looked into this. It appears (after pseudo was written, but) about > 7 or so years ago, all of the man pages were re-written and the > language was changed. I don't think I originally intentionally synchronized it with man pages, I just had the convention of special-casing "path", and then started using "pathname" as a convention for "a name that holds a path but we don't want to do the magic on it". -s ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [OE-core] [PATCH 2/2] pseudo: Fix statx function usage 2020-10-12 19:08 ` [OE-core] " Seebs 2020-10-12 19:14 ` Mark Hatle @ 2020-10-12 20:25 ` Richard Purdie 1 sibling, 0 replies; 6+ messages in thread From: Richard Purdie @ 2020-10-12 20:25 UTC (permalink / raw) To: Seebs; +Cc: openembedded-core On Mon, 2020-10-12 at 14:08 -0500, Seebs wrote: > On Wed, 7 Oct 2020 17:20:18 +0100 > "Richard Purdie" <richard.purdie@linuxfoundation.org> wrote: > > > +There is magic in the posts where specific variable names have > > specific +magic. For that magic to work, "path" needs to be used > > not > > "pathname" as +is currently there. Fix this, which fixes path > > issues > > on systems using +statx (Ubuntu 20.04 in particular). > > So, usually if "pathname" is used, it's intentional to suppress the > special magic behavior. In this case I added the statx wrapper and didn't realise there was magic related to the name "path", so its a bug I introduced. This patch fixes my mistake. I think these are only on the oe-core branch, not master. > Usually, but not always. In this case, the man > page does use "pathname" and sometimes that means it's just a cut and > paste error. Its my own copy and paste bug :) > My man page also says there's no glibc wrapper for statx, making me > slightly surprised that a wrapper for it is needed/relevant. Its definitely being used by Ubuntu 20.04 systems as we're findining bugs related to it! Thanks for the review! Cheers, Richard ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-10-12 20:25 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-10-07 16:20 [PATCH 1/2] pseudo: do not expand symlinks in /proc Richard Purdie 2020-10-07 16:20 ` [PATCH 2/2] pseudo: Fix statx function usage Richard Purdie 2020-10-12 19:08 ` [OE-core] " Seebs 2020-10-12 19:14 ` Mark Hatle 2020-10-12 19:21 ` Seebs 2020-10-12 20:25 ` Richard Purdie
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.