On Mon, Aug 24, 2009 at 01:11:54PM +0200, Corentin Chary wrote:
> +static void
> +ubi_probe_all(blkid_cache cache, int only_if_new)
> +{
> + const char **dirname;
> +
> + for (dirname = dirlist; *dirname; dirname++) {
> + DBG(DEBUG_DEVNAME, printf("probing UBI volumes under %s\n",
> + *dirname));
> +
> + DIR *dir;
> + struct dirent *iter;
> +
> + dir = opendir(*dirname);
> + if (dir == NULL)
> + continue ;
> +
> + while ((iter = readdir(dir)) != NULL) {
> + char *name, *device;
> + struct stat st;
> + dev_t dev;
> +
> + name = iter->d_name;
> +
> + if (!strcmp(name, ".") || !strcmp(name, "..") ||
> + !strstr(name, "ubi"))
> + continue;
> + if (!strcmp(name, "ubi_ctrl"))
> + continue;
> + device = malloc(strlen(*dirname) + strlen(name) + 2);
> + if (!device)
> + break ;
> + sprintf(device, "%s/%s", *dirname, name);
> + if (stat(device, &st))
just for the record, here is possible memory leak, should be: free(device);
> + break ;
I did some changes (see patch below) to all places where we scan
directories in libblkid. From my point of view is better to use
fstatat() than malloc() + sprintf() for paths. It also seems that we
can a little reduce overhead of readdir() loops by dirent->d_type.
(Yes, I know that Linux has cheap stat().)
Karel
> +
> + if (!(st.st_rdev & 0xFF)) { // It's an UBI Device
> + free(device);
> + continue ;
> + }
> + dev = st.st_rdev;
> + DBG(DEBUG_DEVNAME, printf("UBI vol %s: devno 0x%04X\n",
> + device,
> + (int) dev));
> + probe_one(cache, name, dev, BLKID_PRI_UBI,
> + only_if_new);
> + free(device);
> + }
> + closedir(dir);
> + }
>From f38fd19d2b0367f5f4517ba30a8964802c7ce1ab Mon Sep 17 00:00:00 2001
From: Karel Zak <kzak@xxxxxxxxxx>
Date: Fri, 25 Sep 2009 14:40:23 +0200
Subject: [PATCH] libblkid: use fstatat(), improve readdir() usage
* fix possible memory leak in ubi_probe_all()
* use dirent->d_type if available (this is tricky, because d_type
is not supported on all systems and some filesystems returns
DT_UNKNOWN). The dirent->d_type allows to avoid unnecessary stat()
calls.
* use fstatat() if available -- allows to avoid malloc() and
sprintf("%s/%s", dirname, dirent->d_name)
Signed-off-by: Karel Zak <kzak@xxxxxxxxxx>
---
TODO | 2 -
shlibs/blkid/src/blkidP.h | 5 ++
shlibs/blkid/src/devname.c | 34 +++++++---------
shlibs/blkid/src/devno.c | 96 ++++++++++++++++++++++++++++++++++++-------
4 files changed, 100 insertions(+), 37 deletions(-)
diff --git a/TODO b/TODO
index 44fb2f8..bf2c5f9 100644
--- a/TODO
+++ b/TODO
@@ -26,8 +26,6 @@ libblkid
[ -- Christoph Hellwig, 16 Feb 2009 ]
(1) http://git.kernel.org/?p=fs/xfs/xfsprogs-dev.git;a=tree;f=libdisk;
- - use fstatat() in blkid__scan_dir()
-
- add values:
FSSIZE -- filesystem size (klibc requirement)
diff --git a/shlibs/blkid/src/blkidP.h b/shlibs/blkid/src/blkidP.h
index 68116c0..d09529e 100644
--- a/shlibs/blkid/src/blkidP.h
+++ b/shlibs/blkid/src/blkidP.h
@@ -17,6 +17,8 @@
#define CONFIG_BLKID_DEBUG 1
#include <sys/types.h>
+#include <dirent.h>
+#include <sys/stat.h>
#include <stdio.h>
#include <stdarg.h>
@@ -279,6 +281,9 @@ struct blkid_struct_cache
extern char *blkid_strdup(const char *s);
extern char *blkid_strndup(const char *s, const int length);
+extern char *blkid_strconcat(const char *a, const char *b, const char *c);
+extern int blkid_fstatat(DIR *dir, const char *dirname, const char *filename,
+ struct stat *st, int nofollow);
#define BLKID_CACHE_FILE "/etc/blkid.tab"
#define BLKID_CONFIG_FILE "/etc/blkid.conf"
diff --git a/shlibs/blkid/src/devname.c b/shlibs/blkid/src/devname.c
index 1acb9d5..fe0d1b3 100644
--- a/shlibs/blkid/src/devname.c
+++ b/shlibs/blkid/src/devname.c
@@ -407,35 +407,31 @@ ubi_probe_all(blkid_cache cache, int only_if_new)
continue ;
while ((iter = readdir(dir)) != NULL) {
- char *name, *device;
+ char *name;
struct stat st;
dev_t dev;
name = iter->d_name;
-
+#ifdef _DIRENT_HAVE_D_TYPE
+ if (iter->d_type != DT_UNKNOWN &&
+ iter->d_type != DT_CHR && iter->d_type != DT_LNK)
+ continue;
+#endif
if (!strcmp(name, ".") || !strcmp(name, "..") ||
!strstr(name, "ubi"))
continue;
if (!strcmp(name, "ubi_ctrl"))
continue;
- device = malloc(strlen(*dirname) + strlen(name) + 2);
- if (!device)
- break ;
- sprintf(device, "%s/%s", *dirname, name);
- if (stat(device, &st))
- break ;
-
- if (!(st.st_rdev & 0xFF)) { // It's an UBI Device
- free(device);
- continue ;
- }
+ if (blkid_fstatat(dir, *dirname, name, &st, 0))
+ continue;
+
dev = st.st_rdev;
- DBG(DEBUG_DEVNAME, printf("UBI vol %s: devno 0x%04X\n",
- device,
- (int) dev));
- probe_one(cache, name, dev, BLKID_PRI_UBI,
- only_if_new);
- free(device);
+
+ if (!S_ISCHR(st.st_mode) || !minor(dev))
+ continue;
+ DBG(DEBUG_DEVNAME, printf("UBI vol %s/%s: devno 0x%04X\n",
+ *dirname, name, (int) dev));
+ probe_one(cache, name, dev, BLKID_PRI_UBI, only_if_new);
}
closedir(dir);
}
diff --git a/shlibs/blkid/src/devno.c b/shlibs/blkid/src/devno.c
index 8518f9f..11f212e 100644
--- a/shlibs/blkid/src/devno.c
+++ b/shlibs/blkid/src/devno.c
@@ -30,6 +30,7 @@
#if HAVE_SYS_MKDEV_H
#include <sys/mkdev.h>
#endif
+#include <fcntl.h>
#include "blkidP.h"
#include "pathnames.h"
@@ -57,17 +58,68 @@ char *blkid_strdup(const char *s)
return blkid_strndup(s, 0);
}
+char *blkid_strconcat(const char *a, const char *b, const char *c)
+{
+ char *res, *p;
+ size_t len, al, bl, cl;
+
+ al = a ? strlen(a) : 0;
+ bl = b ? strlen(b) : 0;
+ cl = c ? strlen(c) : 0;
+
+ len = al + bl + cl;
+ if (!len)
+ return NULL;
+ p = res = malloc(len + 1);
+ if (!res)
+ return NULL;
+ if (al) {
+ memcpy(p, a, al);
+ p += al;
+ }
+ if (bl) {
+ memcpy(p, b, bl);
+ p += bl;
+ }
+ if (cl) {
+ memcpy(p, c, cl);
+ p += cl;
+ }
+ *p = '\0';
+ return res;
+}
+
+int blkid_fstatat(DIR *dir, const char *dirname, const char *filename,
+ struct stat *st, int nofollow)
+{
+#ifdef HAVE_FSTATAT
+ return fstatat(dirfd(dir), filename, st,
+ nofollow ? AT_SYMLINK_NOFOLLOW : 0);
+#else
+ char device[PATH_MAX];
+ int len;
+
+ len = snprintf(device, sizeof(device), "%s/%s", *dirname, name);
+ if (len < 0 || len + 1 > sizeof(path))
+ return -1;
+
+ return nofollow ? lstat(device, st) : stat(device, st);
+#endif
+}
+
/*
* This function adds an entry to the directory list
*/
-static void add_to_dirlist(const char *name, struct dir_list **list)
+static void add_to_dirlist(const char *dir, const char *subdir,
+ struct dir_list **list)
{
struct dir_list *dp;
dp = malloc(sizeof(struct dir_list));
if (!dp)
return;
- dp->name = blkid_strdup(name);
+ dp->name = subdir ? blkid_strconcat(dir, "/", subdir) :
+ blkid_strdup(dir);
if (!dp->name) {
free(dp);
return;
@@ -96,36 +148,48 @@ void blkid__scan_dir(char *dirname, dev_t devno, struct dir_list **list,
{
DIR *dir;
struct dirent *dp;
- char path[1024];
- int dirlen;
struct stat st;
if ((dir = opendir(dirname)) == NULL)
return;
- dirlen = strlen(dirname) + 2;
+
while ((dp = readdir(dir)) != 0) {
- if (dirlen + strlen(dp->d_name) >= sizeof(path))
+#ifdef _DIRENT_HAVE_D_TYPE
+ if (dp->d_type != DT_UNKNOWN && dp->d_type != DT_BLK &&
+ dp->d_type != DT_LNK && dp->d_type != DT_DIR)
continue;
-
+#endif
if (dp->d_name[0] == '.' &&
((dp->d_name[1] == 0) ||
((dp->d_name[1] == '.') && (dp->d_name[2] == 0))))
continue;
- sprintf(path, "%s/%s", dirname, dp->d_name);
- if (stat(path, &st) < 0)
+ if (blkid_fstatat(dir, dirname, dp->d_name, &st, 0))
continue;
if (S_ISBLK(st.st_mode) && st.st_rdev == devno) {
- *devname = blkid_strdup(path);
+ *devname = blkid_strconcat(dirname, "/", dp->d_name);
DBG(DEBUG_DEVNO,
- printf("found 0x%llx at %s (%p)\n", (long long)devno,
- path, *devname));
+ printf("found 0x%llx at %s\n", (long long)devno,
+ *devname));
break;
}
- if (list && S_ISDIR(st.st_mode) && !lstat(path, &st) &&
- S_ISDIR(st.st_mode))
- add_to_dirlist(path, list);
+
+ if (!list || !S_ISDIR(st.st_mode))
+ continue;
+
+ /* add subdirectory (but not symlink) to the list */
+#ifdef _DIRENT_HAVE_D_TYPE
+ if (dp->d_type == DT_LNK)
+ continue;
+ if (dp->d_type == DT_UNKNOWN)
+#endif
+ {
+ if (blkid_fstatat(dir, dirname, dp->d_name, &st, 1) ||
+ !S_ISDIR(st.st_mode))
+ continue; /* symlink or lstat() failed */
+ }
+ add_to_dirlist(dirname, dp->d_name, list);
}
closedir(dir);
return;
@@ -161,7 +225,7 @@ char *blkid_devno_to_devname(dev_t devno)
* importance, since we are using a stack...
*/
for (dir = devdirs; *dir; dir++)
- add_to_dirlist(*dir, &list);
+ add_to_dirlist(*dir, NULL, &list);
while (list) {
struct dir_list *current = list;
--
1.6.2.5
--
To unsubscribe from this list: send the line "unsubscribe util-linux-ng" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
