[pseudo] pseudo_fix_path: do not expand symlinks in /proc

[pseudo] pseudo_fix_path: do not expand symlinks in /proc

[Sakib Sajal]

From: Matt Cowell <matt.cowell@nokia.com>

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:
- run_tests: all tests passed (3 tests check the new code path).
             Checked test output to make sure the new codepath gets executed.
- perftest: measured time before and after applying the patch
            had insignificant differences (roughly ~1%)
- world build: completed without warning/errors

Signed-off-by: Sakib Sajal <sakib.sajal@windriver.com>
---
 pseudo_util.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/pseudo_util.c b/pseudo_util.c
index b6980c2..b9de81e 100644
--- a/pseudo_util.c
+++ b/pseudo_util.c
@@ -29,6 +29,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;
@@ -678,6 +683,18 @@ 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
+		 * check if newpath starts with "/proc/"
+		 * strlen of "/proc/" = 6
+		 */
+		if (is_link && (strncmp("/proc/", newpath, 6) == 0)) {
+			pseudo_debug(PDBGF_PATH | PDBGF_VERBOSE,
+				"pae: '%s' is procfs symlink, not expanding\n",
+				newpath);
+			is_link = 0;
+		}
+
 		if (link_recursion >= PSEUDO_MAX_LINK_RECURSION && is_link) {
 			pseudo_diag("link recursion too deep, not expanding path '%s'.\n", newpath);
 			is_link = 0;
-- 
2.25.1

Re: [OE-core] [pseudo] pseudo_fix_path: do not expand symlinks in /proc

[Richard Purdie]

On Thu, 2021-11-11 at 05:11 -0500, Sakib Sajal wrote:
> From: Matt Cowell <matt.cowell@nokia.com>
> 
> 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:
> - run_tests: all tests passed (3 tests check the new code path).
>              Checked test output to make sure the new codepath gets executed.
> - perftest: measured time before and after applying the patch
>             had insignificant differences (roughly ~1%)
> - world build: completed without warning/errors
> 
> Signed-off-by: Sakib Sajal <sakib.sajal@windriver.com>
> ---
>  pseudo_util.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/pseudo_util.c b/pseudo_util.c
> index b6980c2..b9de81e 100644
> --- a/pseudo_util.c
> +++ b/pseudo_util.c
> @@ -29,6 +29,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;
> @@ -678,6 +683,18 @@ 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
> +		 * check if newpath starts with "/proc/"
> +		 * strlen of "/proc/" = 6
> +		 */
> +		if (is_link && (strncmp("/proc/", newpath, 6) == 0)) {
> +			pseudo_debug(PDBGF_PATH | PDBGF_VERBOSE,
> +				"pae: '%s' is procfs symlink, not expanding\n",
> +				newpath);
> +			is_link = 0;
> +		}
> +
>  		if (link_recursion >= PSEUDO_MAX_LINK_RECURSION && is_link) {
>  			pseudo_diag("link recursion too deep, not expanding path '%s'.\n", newpath);
>  			is_link = 0;

I merged this into the pseudo repo and put an updated pseudo recipe into a
master-next build. This resulted in issues:

https://autobuilder.yoctoproject.org/typhoon/#/builders/122/builds/485/steps/13/logs/stdio
https://autobuilder.yoctoproject.org/typhoon/#/builders/116/builds/961
https://autobuilder.yoctoproject.org/typhoon/#/builders/118/builds/922

I've stopped the build since it is clear there is something wrong.

Cheers,

Richard

Re: [OE-core] [pseudo] pseudo_fix_path: do not expand symlinks in /proc

[Richard Purdie]

On Mon, 2021-10-25 at 14:03 -0400, Sakib Sajal wrote:
> From: Matt Cowell <matt.cowell@nokia.com>
> 
> 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:
> - run_tests: all tests passed. Checked the test output to make
>              sure the new codepath gets executed.
> - perftest: measured time before and after applying the patch
>             had insignificant differences (roughly ~1%)
> - world build: completed without warning/errors
> 
> 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 b6980c2..b35a34e 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;
> @@ -678,6 +685,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);
> +			}
> +		}
> +


Above this code, there is a call:

	if (!pseudo_real_lstat || (pseudo_real_lstat(newpath, buf) == -1)) {

so we do have a statfs structure populated. Can we test buf.f_type ==
PROC_SUPER_MAGIC on that instead of calling open()? Or does that not contain the
right information?

Cheers,

Richard