On Thu, 14 Apr 2022 17:52:37 -0500, joshua stein wrote:
> login_fbtab(3) supports wildcards but only for every file in a
> directory (/path/*).
>
> This makes it use glob(3) so it can also support more specific
> wildcards like /path/file*
Yes, it is better to use glob(3) than something custom.
Comments inline.
- todd
> diff --git lib/libutil/login_fbtab.c lib/libutil/login_fbtab.c
> index 5eacf4f65ff..39621a0cde4 100644
> --- lib/libutil/login_fbtab.c
> +++ lib/libutil/login_fbtab.c
> @@ -61,8 +61,8 @@
> #include <sys/stat.h>
>
> #include <errno.h>
> -#include <dirent.h>
> #include <limits.h>
> +#include <glob.h>
> #include <paths.h>
> #include <stdio.h>
> #include <stdlib.h>
> @@ -134,49 +134,32 @@ login_fbtab(const char *tty, uid_t uid, gid_t gid)
> static void
> login_protect(const char *path, mode_t mask, uid_t uid, gid_t gid)
> {
> - char buf[PATH_MAX];
> - size_t pathlen = strlen(path);
> - DIR *dir;
> - struct dirent *ent;
> + glob_t g;
> + size_t n;
> + char *gpath;
>
> - if (pathlen >= sizeof(buf)) {
> + if (strlen(path) > PATH_MAX) {
The test should be >= PATH_MAX since PATH_MAX includes space for
the NUL.
> errno = ENAMETOOLONG;
> syslog(LOG_ERR, "%s: %s: %m", _PATH_FBTAB, path);
> return;
> }
>
> - if (strcmp("/*", path + pathlen - 2) != 0) {
> - if (chmod(path, mask) && errno != ENOENT)
> - syslog(LOG_ERR, "%s: chmod(%s): %m", _PATH_FBTAB, path)
> ;
> - if (chown(path, uid, gid) && errno != ENOENT)
> - syslog(LOG_ERR, "%s: chown(%s): %m", _PATH_FBTAB, path)
> ;
> - } else {
> - /*
> - * This is a wildcard directory (/path/to/whatever/ * ).
> - * Make a copy of path without the trailing '*' (but leave
> - * the trailing '/' so we can append directory entries.)
> - */
> - memcpy(buf, path, pathlen - 1);
> - buf[pathlen - 1] = '\0';
> - if ((dir = opendir(buf)) == NULL) {
> - syslog(LOG_ERR, "%s: opendir(%s): %m", _PATH_FBTAB,
> - path);
> - return;
> - }
> + g.gl_offs = 0;
> + if (glob(path, GLOB_DOOFFS, NULL, &g) != 0) {
I don't think you need GLOB_DOOFFS and g.gl_offs here since you are
not reserving slots in gl_pathv. I would just use GLOB_NOSORT
unless you need things to be sorted.
> + if (errno != ENOENT)
> + syslog(LOG_ERR, "%s: glob(%s): %m", _PATH_FBTAB, path);
> + globfree(&g);
> + return;
> + }
>
> - while ((ent = readdir(dir)) != NULL) {
> - if (strcmp(ent->d_name, ".") != 0 &&
> - strcmp(ent->d_name, "..") != 0) {
> - buf[pathlen - 1] = '\0';
> - if (strlcat(buf, ent->d_name, sizeof(buf))
> - >= sizeof(buf)) {
> - errno = ENAMETOOLONG;
> - syslog(LOG_ERR, "%s: %s: %m",
> - _PATH_FBTAB, path);
> - } else
> - login_protect(buf, mask, uid, gid);
> - }
> - }
> - closedir(dir);
> + for (n = 0; n < g.gl_matchc; n++) {
> + gpath = g.gl_pathv[n];
> +
> + if (chmod(gpath, mask) && errno != ENOENT)
> + syslog(LOG_ERR, "%s: chmod(%s): %m", _PATH_FBTAB, gpath
> );
> + if (chown(gpath, uid, gid) && errno != ENOENT)
> + syslog(LOG_ERR, "%s: chown(%s): %m", _PATH_FBTAB, gpath
> );
> }
> +
> + globfree(&g);
> }
