On Wed, 29 Jun 2016, Todd C. Miller wrote:
> On Wed, 29 Jun 2016 07:05:15 -0600, "Todd C. Miller" wrote:
> > Don't you need to make a copy of dp->d_name? Returning dp->d_name
> > after closedir() seems dangerous.
>
> Since closedir() frees dd_buf this would result in a user after free.
Yep. Too much programming in a garbage collected language recently, I
guess. :-)
Revised diff below.
Philip Guenther
Index: gen/devname.c
===================================================================
RCS file: /data/src/openbsd/src/lib/libc/gen/devname.c,v
retrieving revision 1.12
diff -u -p -r1.12 devname.c
--- gen/devname.c 13 Sep 2015 08:31:47 -0000 1.12
+++ gen/devname.c 5 Jul 2016 05:20:49 -0000
@@ -37,14 +37,13 @@
#include <limits.h>
#include <paths.h>
#include <stdbool.h>
-#include <stdio.h>
#include <string.h>
#include <stdlib.h>
static char *
devname_nodb(dev_t dev, mode_t type)
{
- static char buf[sizeof(_PATH_DEV) + NAME_MAX];
+ static char buf[NAME_MAX + 1];
char *name = NULL;
struct dirent *dp;
struct stat sb;
@@ -52,19 +51,14 @@ devname_nodb(dev_t dev, mode_t type)
if ((dirp = opendir(_PATH_DEV)) == NULL)
return (NULL);
- if (strlcpy(buf, _PATH_DEV, sizeof(buf)) >= sizeof(buf))
- return (NULL);
while ((dp = readdir(dirp)) != NULL) {
if (dp->d_type != DT_UNKNOWN && DTTOIF(dp->d_type) != type)
continue;
- buf[sizeof(_PATH_DEV) - 1] = '\0';
- if (strlcat(buf, dp->d_name, sizeof(buf)) >= sizeof(buf))
- continue;
- if (lstat(buf, &sb) == -1)
- continue;
- if (sb.st_rdev != dev || (sb.st_mode & S_IFMT) != type)
+ if (fstatat(dirfd(dirp), dp->d_name, &sb, AT_SYMLINK_NOFOLLOW)
+ || sb.st_rdev != dev || (sb.st_mode & S_IFMT) != type)
continue;
- name = buf + sizeof(_PATH_DEV) - 1;
+ strlcpy(buf, dp->d_name, sizeof(buf));
+ name = buf;
break;
}
closedir(dirp);
