[PATCH] libdm: fix races with udev

[PATCH] libdm: fix races with udev

[Mikulas Patocka]

libdm: fix races with udev

On newer systems (Debian 6 and newer), udev manages nodes in /dev/mapper
directory. It creates, deletes and renames the nodes according to the
state of the kernel driver.

dmsetup tries to manage nodes in /dev/mapper too, so it can race with
udev. dmsetup checks if the node was created/deleted/renamed with the stat
syscall, and skips the operation if it was. However, if udev
creates/deletes/renames the node after the stat syscall and before the
mknod/unlink/rename syscall, dmsetup reports an error.

These races can be easily provoked by inserting sleep at appropriate
places.

This patch avoids the error messages by ignoring errors that could happen
as a result of the race.

Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>

---
 libdm/libdm-common.c |   10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

Index: lvm2-copy/libdm/libdm-common.c
===================================================================
--- lvm2-copy.orig/libdm/libdm-common.c	2013-09-16 18:24:07.000000000 +0200
+++ lvm2-copy/libdm/libdm-common.c	2013-09-16 19:15:12.000000000 +0200
@@ -966,7 +966,9 @@ static int _add_dev_node(const char *dev
 
 	(void) dm_prepare_selinux_context(path, S_IFBLK);
 	old_mask = umask(0);
-	if (mknod(path, S_IFBLK | mode, dev) < 0) {
+
+	/* The node may already have been created by udev. So ignore EEXIST. */
+	if (mknod(path, S_IFBLK | mode, dev) < 0 && errno != EEXIST) {
 		log_error("%s: mknod for %s failed: %s", path, dev_name, strerror(errno));
 		umask(old_mask);
 		(void) dm_prepare_selinux_context(NULL, 0);
@@ -998,7 +1000,8 @@ static int _rm_dev_node(const char *dev_
 		log_warn("Node %s was not removed by udev. "
 			 "Falling back to direct node removal.", path);
 
-	if (unlink(path) < 0) {
+	/* udev may already have deleted the node. Ignore ENOENT. */
+	if (unlink(path) < 0 && errno != ENOENT) {
 		log_error("Unable to unlink device node for '%s'", dev_name);
 		return 0;
 	}
@@ -1054,7 +1057,8 @@ static int _rename_dev_node(const char *
 			 "Falling back to direct node rename.",
 			 oldpath, newpath);
 
-	if (rename(oldpath, newpath) < 0) {
+	/* udev may already have renamed the node. Ignore ENOENT. */
+	if (rename(oldpath, newpath) < 0 && errno != ENOENT) {
 		log_error("Unable to rename device node from '%s' to '%s'",
 			  old_name, new_name);
 		return 0;

[PATCH] libdm: fix races with udev

[Peter Rajnoha]

On 09/16/2013 09:23 PM, Mikulas Patocka wrote:
> libdm: fix races with udev
> 
> On newer systems (Debian 6 and newer), udev manages nodes in /dev/mapper
> directory. It creates, deletes and renames the nodes according to the
> state of the kernel driver.
> 
> dmsetup tries to manage nodes in /dev/mapper too, so it can race with
> udev. dmsetup checks if the node was created/deleted/renamed with the stat
> syscall, and skips the operation if it was. However, if udev
> creates/deletes/renames the node after the stat syscall and before the
> mknod/unlink/rename syscall, dmsetup reports an error.
> 

These checks are performed after udev is synchronized (the dm_udev_wait call)
and after the udev processing is complete, so we normally shouldn't get into this
problem unless there is some bug in certain version of lvm2/udev rules.

Is this reproducible with upstream code as well or was that with some older version only
that is currently in Debian?

Peter

[PATCH] libdm: fix races with udev

[Mikulas Patocka]

On Thu, 19 Sep 2013, Peter Rajnoha wrote:

> On 09/16/2013 09:23 PM, Mikulas Patocka wrote:
> > libdm: fix races with udev
> > 
> > On newer systems (Debian 6 and newer), udev manages nodes in /dev/mapper
> > directory. It creates, deletes and renames the nodes according to the
> > state of the kernel driver.
> > 
> > dmsetup tries to manage nodes in /dev/mapper too, so it can race with
> > udev. dmsetup checks if the node was created/deleted/renamed with the stat
> > syscall, and skips the operation if it was. However, if udev
> > creates/deletes/renames the node after the stat syscall and before the
> > mknod/unlink/rename syscall, dmsetup reports an error.
> > 
> 
> These checks are performed after udev is synchronized (the dm_udev_wait call)
> and after the udev processing is complete, so we normally shouldn't get into this
> problem unless there is some bug in certain version of lvm2/udev rules.
> 
> Is this reproducible with upstream code as well or was that with some older version only
> that is currently in Debian?
> 
> Peter

This race condition can be easily provoked with this patch. Apply it and 
try dmsetup create/remove/rename.

One Debian 6, races in all three operations create/remove/rename can be 
provoked.

On Debian Sid and on RHEL 6, only races in create/rename can be provoked. 
Race in remove doesn't happen.

Mikulas


Index: LVM2.2.02.102/libdm/libdm-common.c
===================================================================
--- LVM2.2.02.102.orig/libdm/libdm-common.c	2013-09-23 16:51:29.000000000 +0200
+++ LVM2.2.02.102/libdm/libdm-common.c	2013-09-23 19:10:32.028514000 +0200
@@ -964,6 +964,8 @@
 		log_warn("%s not set up by udev: Falling back to direct "
 			 "node creation.", path);
 
+	fprintf(stderr, "sleeping 1\n"); sleep(3);
+
 	(void) dm_prepare_selinux_context(path, S_IFBLK);
 	old_mask = umask(0);
 	if (mknod(path, S_IFBLK | mode, dev) < 0) {
@@ -998,6 +1000,8 @@
 		log_warn("Node %s was not removed by udev. "
 			 "Falling back to direct node removal.", path);
 
+	fprintf(stderr, "sleeping 2\n"); sleep(3);
+
 	if (unlink(path) < 0) {
 		log_error("Unable to unlink device node for '%s'", dev_name);
 		return 0;
@@ -1054,6 +1058,8 @@
 			 "Falling back to direct node rename.",
 			 oldpath, newpath);
 
+	fprintf(stderr, "sleeping 3\n"); sleep(3);
+
 	if (rename(oldpath, newpath) < 0) {
 		log_error("Unable to rename device node from '%s' to '%s'",
 			  old_name, new_name);