From mboxrd@z Thu Jan  1 00:00:00 1970
X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org
X-Spam-Level: 
X-Spam-Status: No, score=-3.3 required=5.0 tests=MAILING_LIST_MULTI,
	RCVD_IN_DNSWL_MED,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL autolearn=ham
	autolearn_force=no version=3.4.4
Received: (qmail 12425 invoked from network); 23 Nov 2020 02:03:50 -0000
Received: from mother.openwall.net (195.42.179.200)
  by inbox.vuxu.org with ESMTPUTF8; 23 Nov 2020 02:03:50 -0000
Received: (qmail 5179 invoked by uid 550); 23 Nov 2020 02:03:45 -0000
Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm
Precedence: bulk
List-Post: <mailto:musl@lists.openwall.com>
List-Help: <mailto:musl-help@lists.openwall.com>
List-Unsubscribe: <mailto:musl-unsubscribe@lists.openwall.com>
List-Subscribe: <mailto:musl-subscribe@lists.openwall.com>
List-ID: <musl.lists.openwall.com>
Reply-To: musl@lists.openwall.com
Received: (qmail 5142 invoked from network); 23 Nov 2020 02:03:37 -0000
MIME-Version: 1.0
Date: Mon, 23 Nov 2020 05:03:25 +0300
From: Alexey Izbyshev <izbyshev@ispras.ru>
To: musl@lists.openwall.com
In-Reply-To: <20201122225619.GR534@brightrain.aerifal.cx>
References: <20201122225619.GR534@brightrain.aerifal.cx>
User-Agent: Roundcube Webmail/1.4.4
Message-ID: <97dd3cf7c69673e5962e9ccd46ea5131@ispras.ru>
X-Sender: izbyshev@ispras.ru
Content-Type: text/plain; charset=US-ASCII;
 format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [musl] realpath without procfs -- should be ready for inclusion

On 2020-11-23 01:56, Rich Felker wrote:
> I originally considered keeping the procfs based version and only
> using the new one as a fallback, but I discovered there are cases
> (involving chroot, namespaces, etc.) where the answer from procfs is
> wrong and validating it requires basically the same procedure as
> implementing it manually (walking and performing readlink on each path
> component).
> 
Pity that the simple and fast procfs-based implementation goes away. Do 
you have any specific example of a wrong answer from procfs at hand, or 
at least a more specific direction to look at than just 
"chroot/namespaces"?

> #define _GNU_SOURCE
> #include <stdlib.h>
> #include <limits.h>
> #include <errno.h>
> #include <unistd.h>
> #include <string.h>
> 
> static inline int at_dotdot(const char *end, size_t len)
> {
>    if (len<2) return 0;
>    if (len>2 && end[-3]!='/') return 0;
>    return end[-1]=='.' && end[-2]=='.';
> }
> 
> char *realpath(const char *restrict filename, char *restrict resolved)
> {
>    char stack[PATH_MAX];
>    char buf[resolved ? 1 : PATH_MAX];
>    char *output = resolved ? resolved : buf;
>    size_t p, q, l, cnt=0;
> 
>    l = strnlen(filename, sizeof stack + 1);

Why + 1?

>    if (!l) {
>        errno = ENOENT;
>        return 0;
>    }
>    if (l >= sizeof stack) goto toolong;
>    p = sizeof stack - l - 1;
>    q = 0;
>    memcpy(stack+p, filename, l+1);
> 
>    while (stack[p]) {
>        int up = 0;
>        if (stack[p] == '/') {
>            q=0;
>            output[q++] = '/';
>            p++;
>            /* Initial // is special. */
>            if (stack[p] == '/' && stack[p+1] != '/') {
>                output[q++] = '/';
>            }
>            while (stack[p] == '/') p++;
>            continue;
>        }
>        char *z = __strchrnul(stack+p, '/');
>        l = z-(stack+p);
>        if (l==1 && stack[p]=='.') {
>            p += l;
>            while (stack[p] == '/') p++;
>            continue;
>        }
>        if (at_dotdot(stack+p+l, l)) {
>            if (q && !at_dotdot(output+q, q)) {
>                while(q && output[q-1]!='/') q--;
>                if (q>1 && (q>2 || output[0]!='/')) q--;
>                p += l;
>                while (stack[p] == '/') p++;
>                continue;
>            }
>            up = 1;
>        }
>        if (q && output[q-1] != '/') {
>            if (!p) goto toolong;
>            stack[--p] = '/';
>            l++;
>        }
>        if (q+l >= PATH_MAX) goto toolong;
>        memcpy(output+q, stack+p, l);
>        output[q+l] = 0;
>        p += l;
>        if (up) goto notlink;
>        ssize_t k = readlink(output, stack, p);
>        if (k==-1) {
>            if (errno == EINVAL) {
> notlink:
>                q += l;
>                while (stack[p] == '/') p++;
>                continue;
>            }
>            return 0;
>        }
>        if (k==p) goto toolong;
>        if (++cnt == SYMLOOP_MAX) {
>            errno = ELOOP;
>            return 0;
>        }
>        p -= k;
>        memmove(stack+p, stack, k);

This isn't always correct if the symlink resolves to "/" because 
stack[p] might be '/', so two slashes will be preserved in the output. 
For example, "root/home" resolves to "//home" (where "root" -> "/").

>    }
> 
>    output[q] = 0;
> 
>    if (output[0] != '/') {
>        if (!getcwd(stack, sizeof stack)) return 0;
>        l = strlen(stack);
>        /* Cancel any initial .. components. */
>        p = 0;
>        while (q-p>=2 && at_dotdot(output+p+2, p+2)) {

This doesn't check that output+p+2 is the end of a path element, which 
is the prerequisite for at_dotdot(). So, for example, "..." resolves 
incorrectly.

>            while(l>1 && stack[l-1]!='/') l--;
>            if (l>1) l--;
>            p += 2;
>            if (p<q) p++;
>        }
>        if (q-p && stack[l-1]!='/') output[--p] = '/';
>        if (l + (q-p) + 1 >= PATH_MAX) goto toolong;
>        memmove(output + l, output + p, q - p + 1);
>        memcpy(output, stack, l);
>    }
> 
>    return resolved ? resolved : strdup(output);
> 
> toolong:
>    errno = ENAMETOOLONG;
>    return 0;
> }
> 

Alexey