mailing list of musl libc
 help / color / mirror / code / Atom feed
* realpath() depends on a mounted /proc to work
@ 2016-06-07  0:27 Lei Zhang
  2016-06-07  1:15 ` Rich Felker
  0 siblings, 1 reply; 7+ messages in thread
From: Lei Zhang @ 2016-06-07  0:27 UTC (permalink / raw)
  To: musl

Hi,

I observed that realpath() doesn't work correctly without a mounted
/proc while experimenting in a chroot system, where musl is the
default libc. OTOH, the same program statically linked against glibc
worked just as expected.

Is the dependence on a mounted /proc intentional? Then what's the
rationale behind it?


Thanks,
Lei


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: realpath() depends on a mounted /proc to work
  2016-06-07  0:27 realpath() depends on a mounted /proc to work Lei Zhang
@ 2016-06-07  1:15 ` Rich Felker
  2016-06-07  5:17   ` Luca Barbato
  2016-06-07  5:45   ` Timo Teras
  0 siblings, 2 replies; 7+ messages in thread
From: Rich Felker @ 2016-06-07  1:15 UTC (permalink / raw)
  To: musl

On Tue, Jun 07, 2016 at 08:27:18AM +0800, Lei Zhang wrote:
> Hi,
> 
> I observed that realpath() doesn't work correctly without a mounted
> /proc while experimenting in a chroot system, where musl is the
> default libc. OTOH, the same program statically linked against glibc
> worked just as expected.
> 
> Is the dependence on a mounted /proc intentional? Then what's the
> rationale behind it?

As a whole, musl depends on having /proc; the rationale is simply that
Linux does not expose all the functionality needed to implement POSIX
without /proc.

As for realpath, it could be done without /proc, but being that musl
already requires /proc for full functionality, and that non-/proc
versions of realpath are significantly heavier, more complex, and less
reliable, simply using /proc for realpath was the natural choice.

If someone wants to do a survey of what functionality actually depends
on /proc, we could document that and also evaluate whether realpath is
sufficiently more "basic" than other things which need /proc that we
might want to consider having a fallback implementation for when /proc
is missign.

Rich


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: realpath() depends on a mounted /proc to work
  2016-06-07  1:15 ` Rich Felker
@ 2016-06-07  5:17   ` Luca Barbato
  2016-06-07  5:45   ` Timo Teras
  1 sibling, 0 replies; 7+ messages in thread
From: Luca Barbato @ 2016-06-07  5:17 UTC (permalink / raw)
  To: musl

On 07/06/16 03:15, Rich Felker wrote:
> As a whole, musl depends on having /proc; the rationale is simply that
> Linux does not expose all the functionality needed to implement POSIX
> without /proc.

That would be a boon for those wanting to write an (early and not early)
init system and be sure that the code that would lead to mount /proc
wouldn't have any issue.

lu


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: realpath() depends on a mounted /proc to work
  2016-06-07  1:15 ` Rich Felker
  2016-06-07  5:17   ` Luca Barbato
@ 2016-06-07  5:45   ` Timo Teras
  2016-06-07  7:13     ` FRIGN
  2016-06-08 17:50     ` Rich Felker
  1 sibling, 2 replies; 7+ messages in thread
From: Timo Teras @ 2016-06-07  5:45 UTC (permalink / raw)
  To: Rich Felker; +Cc: musl

On Mon, 6 Jun 2016 21:15:39 -0400
Rich Felker <dalias@libc.org> wrote:

> On Tue, Jun 07, 2016 at 08:27:18AM +0800, Lei Zhang wrote:
> > Hi,
> > 
> > I observed that realpath() doesn't work correctly without a mounted
> > /proc while experimenting in a chroot system, where musl is the
> > default libc. OTOH, the same program statically linked against glibc
> > worked just as expected.
> > 
> > Is the dependence on a mounted /proc intentional? Then what's the
> > rationale behind it?  
> 
> As a whole, musl depends on having /proc; the rationale is simply that
> Linux does not expose all the functionality needed to implement POSIX
> without /proc.
> 
> As for realpath, it could be done without /proc, but being that musl
> already requires /proc for full functionality, and that non-/proc
> versions of realpath are significantly heavier, more complex, and less
> reliable, simply using /proc for realpath was the natural choice.
> 
> If someone wants to do a survey of what functionality actually depends
> on /proc, we could document that and also evaluate whether realpath is
> sufficiently more "basic" than other things which need /proc that we
> might want to consider having a fallback implementation for when /proc
> is missign.

I remember we discussed this earlier, as I would prefer to have it for
running scripts in chroot from alpine package manager (without /proc).

This would also fix applications that chroot to isolated place after
startup (e.g. some ftp servers).

Grep says /proc is found to be used in:
 - __synccall - /proc/self/task
 - __procfdname - to generate /proc/self/fd/%d
 - dl_iterate_phdr - /proc/self/exe as name for main executable
 - dynlink - for $ORIGIN (and filtering it out from AT_EXECFN)

__synccall is used basically for setrlimit() and set*uid(). These
generally are done in the application startup, but could become issue
in chrooted multi-threaded programs.

And __procfdname is used in:
 - ttyname_r
 - realpath
 - fexecve
 - fchdir
 - fchown
 - fstat
 - fchmodat
 - fchmod

For these fexecve, ttyname_r and realpath seem to unconditionally
depend on it. For the other f* fd functions it is only the fallback
code for really old kernels.

Given the above, I would vote realpath() to have fallback code. It's
far more widely used, and would fix several chroot usage scenarios.

Btw, one discrepency is that ttyname_r declares:
	char procname[sizeof "/proc/self/fd/" + 3*sizeof(int) + 2];
and all other users:
	char buf[15+3*sizeof(int)];

ttyname_r this allocates one more byte for the buffer and makes it
clear where that size comes from. Perhaps this would be good place for
some internal header that has #define and prototype for __procfdname.

Thanks,
Timo


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: realpath() depends on a mounted /proc to work
  2016-06-07  5:45   ` Timo Teras
@ 2016-06-07  7:13     ` FRIGN
  2016-06-08 17:50     ` Rich Felker
  1 sibling, 0 replies; 7+ messages in thread
From: FRIGN @ 2016-06-07  7:13 UTC (permalink / raw)
  To: musl

On Tue, 7 Jun 2016 08:45:21 +0300
Timo Teras <timo.teras@iki.fi> wrote:

Hey Timo,

> Given the above, I would vote realpath() to have fallback code. It's
> far more widely used, and would fix several chroot usage scenarios.

I second this, given I use realpath() on webservers to get canonical
pathnames. Of course, those webservers are in a chroot environment.
It also took me quite a while to find out what the problem was
(and why the program froze at realpath()).

Cheers

FRIGN

-- 
FRIGN <dev@frign.de>


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: realpath() depends on a mounted /proc to work
  2016-06-07  5:45   ` Timo Teras
  2016-06-07  7:13     ` FRIGN
@ 2016-06-08 17:50     ` Rich Felker
  2016-11-03 17:03       ` Quentin Rameau
  1 sibling, 1 reply; 7+ messages in thread
From: Rich Felker @ 2016-06-08 17:50 UTC (permalink / raw)
  To: musl

On Tue, Jun 07, 2016 at 08:45:21AM +0300, Timo Teras wrote:
> On Mon, 6 Jun 2016 21:15:39 -0400
> Rich Felker <dalias@libc.org> wrote:
> 
> > On Tue, Jun 07, 2016 at 08:27:18AM +0800, Lei Zhang wrote:
> > > Hi,
> > > 
> > > I observed that realpath() doesn't work correctly without a mounted
> > > /proc while experimenting in a chroot system, where musl is the
> > > default libc. OTOH, the same program statically linked against glibc
> > > worked just as expected.
> > > 
> > > Is the dependence on a mounted /proc intentional? Then what's the
> > > rationale behind it?  
> > 
> > As a whole, musl depends on having /proc; the rationale is simply that
> > Linux does not expose all the functionality needed to implement POSIX
> > without /proc.
> > 
> > As for realpath, it could be done without /proc, but being that musl
> > already requires /proc for full functionality, and that non-/proc
> > versions of realpath are significantly heavier, more complex, and less
> > reliable, simply using /proc for realpath was the natural choice.
> > 
> > If someone wants to do a survey of what functionality actually depends
> > on /proc, we could document that and also evaluate whether realpath is
> > sufficiently more "basic" than other things which need /proc that we
> > might want to consider having a fallback implementation for when /proc
> > is missign.
> 
> I remember we discussed this earlier, as I would prefer to have it for
> running scripts in chroot from alpine package manager (without /proc).
> 
> This would also fix applications that chroot to isolated place after
> startup (e.g. some ftp servers).
> 
> Grep says /proc is found to be used in:
>  - __synccall - /proc/self/task
>  - __procfdname - to generate /proc/self/fd/%d
>  - dl_iterate_phdr - /proc/self/exe as name for main executable
>  - dynlink - for $ORIGIN (and filtering it out from AT_EXECFN)
> 
> __synccall is used basically for setrlimit() and set*uid(). These
> generally are done in the application startup, but could become issue
> in chrooted multi-threaded programs.
> 
> And __procfdname is used in:
>  - ttyname_r
>  - realpath
>  - fexecve
>  - fchdir
>  - fchown
>  - fstat
>  - fchmodat
>  - fchmod
> 
> For these fexecve, ttyname_r and realpath seem to unconditionally
> depend on it. For the other f* fd functions it is only the fallback
> code for really old kernels.

This is not quite true. As of at least 3.18 or so, Linux did not
support most of the f* ops (fstat, fchdir, etc.) on O_PATH file
descriptors; it may still lack support. Also the Linux fchmodat
syscall completely lacks the flag argument (contrary to musl's useless
passing of it) so /proc is unconditionally required to support
fchmodat with nonzero flag.

> Given the above, I would vote realpath() to have fallback code. It's
> far more widely used, and would fix several chroot usage scenarios.

That seems reasonable. Fallback code should avoid using significant
additional stack space and should probably only be used if the proc
version fails with ENOENT (proc not mounted) and not on transient
errors, so that it doesn't become a bug surface that could affect
properly configured systems under low-resource conditions.

> Btw, one discrepency is that ttyname_r declares:
> 	char procname[sizeof "/proc/self/fd/" + 3*sizeof(int) + 2];
> and all other users:
> 	char buf[15+3*sizeof(int)];
> 
> ttyname_r this allocates one more byte for the buffer and makes it
> clear where that size comes from. Perhaps this would be good place for
> some internal header that has #define and prototype for __procfdname.

Yes that might be a good idea. BTW I think it's 2 more bytes, not 1.

Rich


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: realpath() depends on a mounted /proc to work
  2016-06-08 17:50     ` Rich Felker
@ 2016-11-03 17:03       ` Quentin Rameau
  0 siblings, 0 replies; 7+ messages in thread
From: Quentin Rameau @ 2016-11-03 17:03 UTC (permalink / raw)
  To: musl

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

Hi,

> > > > I observed that realpath() doesn't work correctly without a mounted
> > > > /proc while experimenting in a chroot system, where musl is the
> > > > default libc. OTOH, the same program statically linked against glibc
> > > > worked just as expected.
[…]
> > Given the above, I would vote realpath() to have fallback code. It's
> > far more widely used, and would fix several chroot usage scenarios.
> 
> That seems reasonable. Fallback code should avoid using significant
> additional stack space and should probably only be used if the proc
> version fails with ENOENT (proc not mounted) and not on transient
> errors, so that it doesn't become a bug surface that could affect
> properly configured systems under low-resource conditions.

I encountered the same issue and after finding this thread I decided to
have a go at implementing a naive realpath fallback.

I tried to follow the coding style of what I saw in musl but I'm not
sure which approach would be preferred so I joint two patches to this
mail, first using pointers, second using array indexes.

I'll gladly rework on this first batch if you're interested in it but
not satisfied yet with it.

Thanks!

[-- Attachment #2: 0001-realpath-add-fallback-processing-without-proc_pointers.patch --]
[-- Type: text/plain, Size: 2619 bytes --]

From 5c2c2717e4bd8cf8a9f816c043e102e317e1d949 Mon Sep 17 00:00:00 2001
From: Quentin Rameau <quinq@fifth.space>
Date: Tue, 1 Nov 2016 12:53:53 +0100
Subject: [PATCH] realpath: add fallback processing without /proc

---
 src/misc/realpath.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 63 insertions(+), 2 deletions(-)

diff --git a/src/misc/realpath.c b/src/misc/realpath.c
index 88c849c..b387b59 100644
--- a/src/misc/realpath.c
+++ b/src/misc/realpath.c
@@ -9,13 +9,48 @@
 
 void __procfdname(char *, unsigned);
 
+int mergepaths(char *restrict absolute, const char *restrict relative)
+{
+	char *a = absolute;
+	const char *r = relative;
+
+	/* if relative is absolute, skip absolute */
+	if (*r == '/') {
+		*a = '/';
+		++r;
+	} else {
+		/* slash terminate absolute if needed */
+		for (; *a; ++a);
+		if (*r && (a[-1] != '/')) *a = '/';
+		else --a;
+	}
+
+	/* resolve . and .. */
+	for (; *r; ++r) {
+		if (*r == '.') {
+			if (r[1] == '/' || !r[1]) continue;
+			if (r[1] == '.' && (r[2] == '/' || !r[2])) {
+				while (a > absolute && *--a != '/');
+				continue;
+			}
+		} else if (*r == '/' && *a == '/') continue;
+		*++a = *r;
+	}
+	/* terminate absolute */
+	if (*a == '/' && a > absolute) --a;
+	*++a = 0;
+
+	return a - absolute;
+}
+
 char *realpath(const char *restrict filename, char *restrict resolved)
 {
 	int fd;
 	ssize_t r;
 	struct stat st1, st2;
 	char buf[15+3*sizeof(int)];
-	char tmp[PATH_MAX];
+	char tmp[PATH_MAX], link[PATH_MAX];
+	char *p;
 
 	if (!filename) {
 		errno = EINVAL;
@@ -27,7 +62,7 @@ char *realpath(const char *restrict filename, char *restrict resolved)
 	__procfdname(buf, fd);
 
 	r = readlink(buf, tmp, sizeof tmp - 1);
-	if (r < 0) goto err;
+	if (r < 0) goto noproc;
 	tmp[r] = 0;
 
 	fstat(fd, &st1);
@@ -37,8 +72,34 @@ char *realpath(const char *restrict filename, char *restrict resolved)
 		goto err;
 	}
 
+end:
 	__syscall(SYS_close, fd);
 	return resolved ? strcpy(resolved, tmp) : strdup(tmp);
+
+noproc:
+	if (*filename != '/')
+		if (syscall(SYS_getcwd, tmp, sizeof(tmp)) < 0) goto err;
+
+	/* concatenate cwd and target */
+	r = mergepaths(tmp, filename);
+
+	if (lstat(tmp, &st1) < 0) goto err;
+	/* resolve link chain if any */
+	while (S_ISLNK(st1.st_mode)) {
+		if ((r = readlink(tmp, link, sizeof(link))) < 0) goto err;
+		link[r] = 0;
+
+		/* remove link name */
+		for (p = tmp; *p; ++p);
+		while (p > tmp && *--p != '/');
+		*p = 0;
+
+		/* merge path and link target */
+		mergepaths(tmp, link);
+		if (lstat(tmp, &st1) < 0) goto err;
+	}
+	goto end;
+
 err:
 	__syscall(SYS_close, fd);
 	return 0;
-- 
2.10.2


[-- Attachment #3: 0001-realpath-add-fallback-processing-without-proc_arrays.patch --]
[-- Type: text/plain, Size: 2581 bytes --]

From 304b193729b4eb5218218a71320cb5c93b1d19ca Mon Sep 17 00:00:00 2001
From: Quentin Rameau <quinq@fifth.space>
Date: Tue, 1 Nov 2016 12:53:53 +0100
Subject: [PATCH] realpath: add fallback processing without /proc

---
 src/misc/realpath.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 63 insertions(+), 2 deletions(-)

diff --git a/src/misc/realpath.c b/src/misc/realpath.c
index 88c849c..e48420d 100644
--- a/src/misc/realpath.c
+++ b/src/misc/realpath.c
@@ -9,13 +9,48 @@
 
 void __procfdname(char *, unsigned);
 
+int mergepaths(char *restrict a, const char *restrict r)
+{
+	size_t i, j;
+	i = j = 0;
+
+	/* if r is absolute, skip a content */
+	if (r[0] == '/') {
+		a[0] = '/';
+		j = 1;
+	} else {
+		/* slash terminate a if needed */
+		i = strlen(a)-1;
+		if (r[0] && (a[i-1] != '/')) a[i] = '/';
+		else --i;
+	}
+
+	/* resolve . and .. */
+	for (; r[j]; ++j) {
+		if (r[j] == '.') {
+			if (r[j+1] == '/' || !r[j+1]) continue;
+			if (r[j+1] == '.' && (r[j+2] == '/' || !r[j+2])) {
+				while (i > 0 && a[--i] != '/');
+				continue;
+			}
+		} else if (r[j] == '/' && a[i] == '/') continue;
+		a[++i] = r[j];
+	}
+	/* terminate a */
+	if (a[i] == '/' && i > 0) --i;
+	a[++i] = 0;
+
+	return i;
+}
+
 char *realpath(const char *restrict filename, char *restrict resolved)
 {
 	int fd;
 	ssize_t r;
+	size_t i;
 	struct stat st1, st2;
 	char buf[15+3*sizeof(int)];
-	char tmp[PATH_MAX];
+	char tmp[PATH_MAX], link[PATH_MAX];
 
 	if (!filename) {
 		errno = EINVAL;
@@ -27,7 +62,7 @@ char *realpath(const char *restrict filename, char *restrict resolved)
 	__procfdname(buf, fd);
 
 	r = readlink(buf, tmp, sizeof tmp - 1);
-	if (r < 0) goto err;
+	if (r < 0) goto noproc;
 	tmp[r] = 0;
 
 	fstat(fd, &st1);
@@ -37,8 +72,34 @@ char *realpath(const char *restrict filename, char *restrict resolved)
 		goto err;
 	}
 
+end:
 	__syscall(SYS_close, fd);
 	return resolved ? strcpy(resolved, tmp) : strdup(tmp);
+
+noproc:
+	if (*filename != '/')
+		if (syscall(SYS_getcwd, tmp, sizeof(tmp)) < 0) goto err;
+
+	/* concatenate cwd and target */
+	r = mergepaths(tmp, filename);
+
+	if (lstat(tmp, &st1) < 0) goto err;
+	/* resolve link chain if any */
+	while (S_ISLNK(st1.st_mode)) {
+		if ((r = readlink(tmp, link, sizeof(link))) < 0) goto err;
+		link[r] = 0;
+
+		/* remove link name */
+		i = strlen(tmp) - 1;
+		while (i > 0 && tmp[--i] != '/');
+		tmp[i] = 0;
+
+		/* merge path and link target */
+		mergepaths(tmp, link);
+		if (lstat(tmp, &st1) < 0) goto err;
+	}
+	goto end;
+
 err:
 	__syscall(SYS_close, fd);
 	return 0;
-- 
2.10.2


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2016-11-03 17:03 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-07  0:27 realpath() depends on a mounted /proc to work Lei Zhang
2016-06-07  1:15 ` Rich Felker
2016-06-07  5:17   ` Luca Barbato
2016-06-07  5:45   ` Timo Teras
2016-06-07  7:13     ` FRIGN
2016-06-08 17:50     ` Rich Felker
2016-11-03 17:03       ` Quentin Rameau

Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/musl/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).