Re: [hackers] [sbase] [PATCH] ls: allow listing contents of directories with +r-x

From: Michael Forney <mforney_AT_mforney.org>
Date: Fri, 20 Oct 2017 13:28:30 -0700

Hi David,

Thanks for the report and patch.

On 2017-10-17, David Phillips <david_AT_sighup.nz> wrote:
> chdir()ing into a directory with +r-x fails, so we should manually use the
> directory name as a prefix rather than chdir()ing into it.

I wonder if we could make use of directory file descriptors and the
*at functions instead of building up a prefix.

> Also adds new parameters to mkent for the prefix, and for dictating whether
> or not a permission denied error when stat()ing shall be fatal or not. This
> allows errors stat()ing children of `foo` to be reported and non-fatal,
> while
> a permission denied error on `foo` itself would be fatal.

Can you explain the reasoning behind this? Is this behavior
standardized, or consistent with other ls(1) implementations?

> ---
> ls.c | 33 +++++++++++++++++++++++----------
> 1 file changed, 23 insertions(+), 10 deletions(-)
>
> diff --git a/ls.c b/ls.c
> index b716aba..c01a277 100644
> --- a/ls.c
> +++ b/ls.c
> _AT_@ -10,6 +10,7 @@
> #include <string.h>
> #include <time.h>
> #include <unistd.h>
> +#include <errno.h>
>
> #include "utf.h"
> #include "util.h"
> _AT_@ -58,15 +59,29 @@ static int showdirs;
> static void ls(const char *, const struct entry *, int);
>
> static void
> -mkent(struct entry *ent, char *path, int dostat, int follow)
> +mkent(struct entry *ent, char *prefix, char *name, int dostat, int follow,
> + int fatal)
> {
> struct stat st;
> + char path[PATH_MAX];
>
> - ent->name = path;
> + ent->name = name;
> if (!dostat)
> return;
> - if ((follow ? stat : lstat)(path, &st) < 0)
> - eprintf("%s %s:", follow ? "stat" : "lstat", path);
> +
> + if (prefix != NULL)
> + snprintf(path, PATH_MAX, "%s/%s", prefix, name);

You need to check whether this returned an error, or the result was truncated.

> + else
> + strncpy(path, name, PATH_MAX);

strncpy is almost never what you want. It does not null-terminate the
string if it doesn't fit in the destination, and if it is shorter than
PATH_MAX, the remaining buffer will be filled with zeros.

> +
> + if ((follow ? stat : lstat)(path, &st) < 0) {
> + if (errno == EACCES && !fatal) {
> + weprintf("%s %s:", follow ? "stat" : "lstat", path);
> + return;
> + } else {
> + eprintf("%s %s:", follow ? "stat" : "lstat", path);
> + }
> + }
> ent->mode = st.st_mode;
> ent->nlink = st.st_nlink;
> ent->uid = st.st_uid;
> _AT_@ -252,8 +267,6 @@ lsdir(const char *path, const struct entry *dir)
> weprintf("opendir %s%s:", path, dir->name);
> return;
> }
> - if (chdir(dir->name) < 0)
> - eprintf("chdir %s:", dir->name);
>
> while ((d = readdir(dp))) {
> if (d->d_name[0] == '.' && !aflag && !Aflag)
> _AT_@ -264,8 +277,8 @@ lsdir(const char *path, const struct entry *dir)
> continue;
>
> ents = ereallocarray(ents, ++n, sizeof(*ents));
> - mkent(&ents[n - 1], estrdup(d->d_name), Fflag || iflag ||
> - lflag || pflag || Rflag || sort, Lflag);
> + mkent(&ents[n - 1], dir->name, estrdup(d->d_name), Fflag || iflag ||
> + lflag || pflag || Rflag || sort, Lflag, 0);
> }
>
> closedir(dp);
> _AT_@ -446,7 +459,7 @@ main(int argc, char *argv[])
> case 0: /* fallthrough */
> *--argv = ".", ++argc;
> case 1:
> - mkent(&ent, argv[0], 1, Hflag || Lflag);
> + mkent(&ent, NULL, argv[0], 1, Hflag || Lflag, 1);
> ls("", &ent, (!dflag && S_ISDIR(ent.mode)) ||
> (S_ISLNK(ent.mode) && S_ISDIR(ent.tmode) &&
> !(dflag || Fflag || lflag)));
> _AT_@ -454,7 +467,7 @@ main(int argc, char *argv[])
> break;
> default:
> for (i = ds = fs = 0, fents = dents = NULL; i < argc; ++i) {
> - mkent(&ent, argv[i], 1, Hflag || Lflag);
> + mkent(&ent, NULL, argv[i], 1, Hflag || Lflag, 1);
>
> if ((!dflag && S_ISDIR(ent.mode)) ||
> (S_ISLNK(ent.mode) && S_ISDIR(ent.tmode) &&
> --
> 2.14.2
>
>
Received on Fri Oct 20 2017 - 22:28:30 CEST