On Wednesday 17 June 2009 22:28, Jean Wolter wrote:
> Denys Vlasenko <[email protected]> writes:
>
> > On Wed, Jun 17, 2009 at 3:52 PM, Jean Wolter<[email protected]>
> > wrote:
> >> while using mdev to create device nodes for the capi subsystem mdev
> >> accidently deletes a previously created node. Assume the following
> >> lines in mdev.conf:
> >>
> >> capi 0:0 0660 =capi20
> >> capi([0-9]) 0:0 0660 =capi20.0%1
> >> capi([0-9]*) 0:0 0660 =capi20.%1
> >>
> >> After loading the capi module the kernel generate a number of hotplug
> >> events, starting with capi, followed by capi0-capi32. mdev creates
> >> /dev/capi, and renames it to /dev/capi20.
> >
> > This seems to be ok.
> >
> >> When the event "add tty /class/tty/capi0" comes in, mdev executes
> >>
> >> unlink /dev/capi20
> >> mknod /dev/capi20
> >> rename(/dev/capi20,/dev/capi20.20)
> >>
> >> and erases the original /dev/capi20 device while doing this.
> >
> > This is the corresponding code:
> >
> > /* "Execute" the line we found */
> > if (!delete && major >= 0) {
> > if (ENABLE_FEATURE_MDEV_RENAME)
> > unlink(device_name);
> >
> > You say that /dev/capi20 is unlinked. Why? In event
> > "add tty /class/tty/capi0" device_name == "capi0",
> > thus unlink(device_name) won't delete "capi20". I'm confused.
>
> You have every right to be confused. The events start with "add tty
> /class/tty/capi0" up to /claas/tty/capi32. In the middle of this
> sequence there is the event "add tty /class/tty/capi20"
> device_name="capi20". busybox is configured with
> ENABLE_FEATURE_MDEV_RENAME, so the first thing happens is an
> unlink(device_name); in this case unlink("capi20").
AHA. Thanks for your diagnosis and patch.
Please try this patch. After build, go to testsuite/*
and run "runtest [-v] mdev".
--
vda
diff -d -urpN busybox.1/testsuite/mdev.tests busybox.2/testsuite/mdev.tests
--- busybox.1/testsuite/mdev.tests 2009-06-17 23:00:05.000000000 +0200
+++ busybox.2/testsuite/mdev.tests 2009-06-22 01:26:43.000000000 +0200
@@ -162,6 +162,32 @@ brw-r--r-- 1 0 1 8,0 sda
" \
"" ""
+# continuing to use directory structure from prev test
+rm -rf mdev.testdir/dev/*
+mkdir -p mdev.testdir/sys/class/tty/capi
+echo "191:0" >mdev.testdir/sys/class/tty/capi/dev
+mkdir -p mdev.testdir/sys/class/tty/capi1
+echo "191:1" >mdev.testdir/sys/class/tty/capi1/dev
+mkdir -p mdev.testdir/sys/class/tty/capi20
+echo "191:20" >mdev.testdir/sys/class/tty/capi20/dev
+echo "capi 0:0 0660 =capi20" >mdev.testdir/etc/mdev.conf
+echo "capi([0-9]) 0:0 0660 =capi20.0%1" >>mdev.testdir/etc/mdev.conf
+echo "capi([0-9]*) 0:0 0660 =capi20.%1" >>mdev.testdir/etc/mdev.conf
+# 3rd mdev invocation was deleting capi20
+testing "move rule does not delete stray node" \
+ "\
+ env - PATH=$PATH ACTION=add DEVPATH=/class/tty/capi chroot mdev.testdir /mdev 2>&1;
+ env - PATH=$PATH ACTION=add DEVPATH=/class/tty/capi1 chroot mdev.testdir /mdev 2>&1;
+ env - PATH=$PATH ACTION=add DEVPATH=/class/tty/capi20 chroot mdev.testdir /mdev 2>&1;
+ ls -lnR mdev.testdir/dev | $FILTER_LS" \
+"\
+mdev.testdir/dev:
+crw-rw---- 1 0 0 191,0 capi20
+crw-rw---- 1 0 0 191,1 capi20.01
+crw-rw---- 1 0 0 191,20 capi20.20
+" \
+ "" ""
+
# clean up
rm -rf mdev.testdir
diff -d -urpN busybox.1/util-linux/mdev.c busybox.2/util-linux/mdev.c
--- busybox.1/util-linux/mdev.c 2009-06-17 23:00:05.000000000 +0200
+++ busybox.2/util-linux/mdev.c 2009-06-22 01:22:50.000000000 +0200
@@ -319,22 +319,20 @@ static void make_device(char *path, int
/* "Execute" the line we found */
if (!delete && major >= 0) {
- if (ENABLE_FEATURE_MDEV_RENAME)
- unlink(device_name);
- if (mknod(device_name, mode | type, makedev(major, minor)) && errno != EEXIST)
- bb_perror_msg_and_die("mknod %s", device_name);
+ char *node_name = (char *)device_name;
+ if (ENABLE_FEATURE_MDEV_RENAME && alias)
+ alias = node_name = build_alias(alias, device_name);
+ if (mknod(node_name, mode | type, makedev(major, minor)) && errno != EEXIST)
+ bb_perror_msg_and_die("mknod %s", node_name);
if (major == root_major && minor == root_minor)
- symlink(device_name, "root");
+ symlink(node_name, "root");
if (ENABLE_FEATURE_MDEV_CONF) {
- chmod(device_name, mode);
- chown(device_name, ugid.uid, ugid.gid);
+ chmod(node_name, mode);
+ chown(node_name, ugid.uid, ugid.gid);
}
if (ENABLE_FEATURE_MDEV_RENAME && alias) {
- alias = build_alias(alias, device_name);
- /* move the device, and optionally
- * make a symlink to moved device node */
- if (rename(device_name, alias) == 0 && aliaslink == '>')
- symlink(alias, device_name);
+ if (aliaslink == '>')
+ symlink(node_name, device_name);
free(alias);
}
}
@@ -355,15 +353,15 @@ static void make_device(char *path, int
}
if (delete) {
- unlink(device_name);
- /* At creation time, device might have been moved
- * and a symlink might have been created. Undo that. */
-
+ char *node_name = (char *)device_name;
if (ENABLE_FEATURE_MDEV_RENAME && alias) {
- alias = build_alias(alias, device_name);
- unlink(alias);
- free(alias);
+ alias = node_name = build_alias(alias, device_name);
+ if (aliaslink == '>')
+ unlink(device_name);
}
+ unlink(node_name);
+ if (ENABLE_FEATURE_MDEV_RENAME)
+ free(alias);
}
/* We found matching line.
_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox
