Hi,
> > int util_create_path(struct udev *udev, const char *path)
> > {
> > char p[UTIL_PATH_SIZE];
> > @@ -32,6 +55,8 @@ int util_create_path(struct udev *udev, const char *path)
> > struct stat stats;
> > int ret;
> >
> > + if (util_path_lock())
> > + return -1;
>
> I think the patch missed some call sites. Each create_path() needs to
> be followed by a create_path_done(), but I can see nine calls to
> create_path() calls and only about six to create_path_done().
Hu? I see 8 call sites, and those should all be covered - maybe it's just
because I'm working on a somewhat old tree?
[...]
> I think there's locking recursion here, because we end up calling
> name_index() between create_path() and create_path_done(). Looking
> at the definition of unlock(), that's not going to work.
Below is a new patch that should work with recursive locking (of limited
depth - I hope 2^31-1 levels of recursion is sufficient? :-)
Untested, uncompiled, etc., as usual ...
Florian
diff --git a/libudev/libudev-device-private.c b/libudev/libudev-device-private.c
index 68dc0a5..d386135 100644
--- a/libudev/libudev-device-private.c
+++ b/libudev/libudev-device-private.c
@@ -44,7 +44,8 @@ int udev_device_update_db(struct udev_device *udev_device)
int ret;
devpath_to_db_path(udev, udev_device_get_devpath(udev_device), filename, sizeof(filename));
- util_create_path(udev, filename);
+ if (util_create_path(udev, filename))
+ return -1;
unlink(filename);
udev_list_entry_foreach(list_entry, udev_device_get_properties_list_entry(udev_device))
@@ -86,6 +87,7 @@ file:
f = fopen(filename, "w");
if (f == NULL) {
err(udev, "unable to create db file '%s': %m\n", filename);
+ util_create_path_done();
return -1;
}
info(udev, "created db file for '%s' in '%s'\n", udev_device_get_devpath(udev_device), filename);
@@ -114,6 +116,7 @@ file:
}
fclose(f);
out:
+ util_create_path_done();
return 0;
}
diff --git a/libudev/libudev-queue-private.c b/libudev/libudev-queue-private.c
index 0427b65..f2f3905 100644
--- a/libudev/libudev-queue-private.c
+++ b/libudev/libudev-queue-private.c
@@ -417,6 +417,7 @@ static void update_failed(struct udev_queue_export *udev_queue_export,
udev_selinux_setfscreatecon(udev, filename, S_IFLNK);
symlink(udev_device_get_devpath(udev_device), filename);
udev_selinux_resetfscreatecon(udev);
+ util_create_path_done();
break;
case DEVICE_QUEUED:
diff --git a/libudev/libudev-util-private.c b/libudev/libudev-util-private.c
index c309945..26fbb5c 100644
--- a/libudev/libudev-util-private.c
+++ b/libudev/libudev-util-private.c
@@ -25,6 +25,28 @@
#include "libudev.h"
#include "libudev-private.h"
+static int util_path_lock_cnt,util_path_lock_fd;
+
+static int util_path_lock(void)
+{
+ if (!util_path_lock_cnt) {
+ if ((util_path_lock_fd = open("/dev/udev_util_path_lock", O_WRONLY | O_CREAT)) < 0)
+ return -1;
+ if (flock(util_path_lock_fd, LOCK_EX)) {
+ close(util_create_path_lock_fd);
+ return -1;
+ }
+ util_path_lock_cnt++;
+ }
+ return 0;
+}
+
+static void util_path_unlock(void)
+{
+ if (!--util_path_lock_cnt)
+ close(util_create_path_lock_fd);
+}
+
int util_create_path(struct udev *udev, const char *path)
{
char p[UTIL_PATH_SIZE];
@@ -32,6 +54,8 @@ int util_create_path(struct udev *udev, const char *path)
struct stat stats;
int ret;
+ if (util_path_lock())
+ return -1;
util_strscpy(p, sizeof(p), path);
pos = strrchr(p, '/');
if (pos == NULL)
@@ -47,8 +71,10 @@ int util_create_path(struct udev *udev, const char *path)
if (stat(p, &stats) == 0 && (stats.st_mode & S_IFMT) == S_IFDIR)
return 0;
- if (util_create_path(udev, p) != 0)
+ if (util_create_path(udev, p) != 0) {
+ util_path_unlock();
return -1;
+ }
dbg(udev, "mkdir '%s'\n", p);
udev_selinux_setfscreatecon(udev, p, S_IFDIR|0755);
@@ -60,9 +86,15 @@ int util_create_path(struct udev *udev, const char *path)
if (ret == 0)
return 0;
+ util_path_unlock();
return -1;
}
+void util_create_path_done(void)
+{
+ util_path_unlock();
+}
+
int util_delete_path(struct udev *udev, const char *path)
{
char p[UTIL_PATH_SIZE];
@@ -75,6 +107,8 @@ int util_delete_path(struct udev *udev, const char *path)
if (pos == p || pos == NULL)
return 0;
+ if (util_path_lock())
+ return 0;
while (1) {
*pos = '\0';
pos = strrchr(p, '/');
@@ -88,13 +122,13 @@ int util_delete_path(struct udev *udev, const char *path)
if (errno == ENOENT)
retval = 0;
if (retval) {
- if (errno == ENOTEMPTY)
- return 0;
- err(udev, "rmdir(%s) failed: %m\n", p);
+ if (errno != ENOTEMPTY)
+ err(udev, "rmdir(%s) failed: %m\n", p);
break;
}
dbg(udev, "removed '%s'\n", p);
}
+ util_path_unlock();
return 0;
}
diff --git a/udev/udev-node.c b/udev/udev-node.c
index d73bc6c..4124485 100644
--- a/udev/udev-node.c
+++ b/udev/udev-node.c
@@ -46,10 +46,12 @@ static int name_index(struct udev *udev, const char *devpath, const char *name,
if (add) {
dbg(udev, "creating index: '%s'\n", filename);
- util_create_path(udev, filename);
- fd = open(filename, O_WRONLY|O_TRUNC|O_CREAT, 0644);
- if (fd > 0)
- close(fd);
+ if (!util_create_path(udev, filename)) {
+ fd = open(filename, O_WRONLY|O_TRUNC|O_CREAT, 0644);
+ util_create_path_done();
+ if (fd > 0)
+ close(fd);
+ }
} else {
dbg(udev, "removing index: '%s'\n", filename);
unlink(filename);
@@ -354,8 +356,10 @@ static int update_link(struct udev_device *dev, const char *slink)
/* create symlink to the target with the highest priority */
info(udev, "'%s' with target '%s' has the highest priority %i, create it\n", slink, target, priority);
- util_create_path(udev, slink);
- node_symlink(udev, target, slink);
+ if (!util_create_path(udev, slink)) {
+ node_symlink(udev, target, slink);
+ util_create_path_done();
+ }
out:
return rc;
}
@@ -424,7 +428,8 @@ int udev_node_add(struct udev_device *dev, mode_t mode, uid_t uid, gid_t gid)
major(udev_device_get_devnum(dev)), minor(udev_device_get_devnum(dev)),
mode, uid, gid);
- util_create_path(udev, udev_device_get_devnode(dev));
+ if (util_create_path(udev, udev_device_get_devnode(dev)))
+ return -1;
if (udev_node_mknod(dev, NULL, makedev(0,0), mode, uid, gid) != 0) {
err = -1;
goto exit;
@@ -456,6 +461,7 @@ int udev_node_add(struct udev_device *dev, mode_t mode, uid_t uid, gid_t gid)
update_link(dev, udev_list_entry_get_name(list_entry));
}
exit:
+ util_create_path_done();
return err;
}
diff --git a/udev/udev-rules.c b/udev/udev-rules.c
index 54a54c3..d8f3e7c 100644
--- a/udev/udev-rules.c
+++ b/udev/udev-rules.c
@@ -1729,10 +1729,12 @@ struct udev_rules *udev_rules_new(struct udev *udev, int resolve_names)
/* read dynamic/temporary rules */
util_strscpyl(filename, sizeof(filename), udev_get_dev_path(udev), "/.udev/rules.d", NULL);
if (stat(filename, &statbuf) != 0) {
- util_create_path(udev, filename);
- udev_selinux_setfscreatecon(udev, filename, S_IFDIR|0755);
- mkdir(filename, 0755);
- udev_selinux_resetfscreatecon(udev);
+ if (!util_create_path(udev, filename)) {
+ udev_selinux_setfscreatecon(udev, filename, S_IFDIR|0755);
+ mkdir(filename, 0755);
+ udev_selinux_resetfscreatecon(udev);
+ util_create_path_done();
+ }
}
udev_list_init(&sort_list);
add_matching_files(udev, &sort_list, filename, ".rules");
diff --git a/udev/udev-watch.c b/udev/udev-watch.c
index 5a49c96..744d9cd 100644
--- a/udev/udev-watch.c
+++ b/udev/udev-watch.c
@@ -124,9 +124,11 @@ void udev_watch_begin(struct udev *udev, struct udev_device *dev)
}
snprintf(filename, sizeof(filename), "%s/.udev/watch/%d", udev_get_dev_path(udev), wd);
- util_create_path(udev, filename);
- unlink(filename);
- symlink(udev_device_get_devpath(dev), filename);
+ if (!util_create_path(udev, filename)) {
+ unlink(filename);
+ symlink(udev_device_get_devpath(dev), filename);
+ util_create_path_done();
+ }
udev_device_set_watch_handle(dev, wd);
}
--
To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
