mdev accidently erases previously created nodes while trying to rename nodes

[Jean Wolter]

Denys Vlasenko <vda.linux at googlemail.com> writes:

> On Wed, Jun 17, 2009 at 3:52 PM, Jean Wolter<jw5 at os.inf.tu-dresden.de> wrote:
>> 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:
> ...
> Please describe how failure happens on the code level.

event: add tty /class/tty/capi20
- device_name="capi20"
- alias="capi20.20"
- aliaslink='='

>                 /* "Execute" the line we found */
>                 if (!delete && major >= 0) {
>                         if (ENABLE_FEATURE_MDEV_RENAME)
>                                 unlink(device_name);

unlink(device_name) - unlink("capi20") - we delete an existing node

>                         if (mknod(device_name, mode | type, makedev(major, minor)) && errno != EEXIST)
>                                 bb_perror_msg_and_die("mknod %s", device_name);

mknod(device_name) - mknod("capi20") - we create a new node "capi20"

>                         if (major == root_major && minor == root_minor)
>                                 symlink(device_name, "root");
>                         if (ENABLE_FEATURE_MDEV_CONF) {
>                                 chmod(device_name, mode);
>                                 chown(device_name, ugid.uid, ugid.gid);
>                         }

set owner and permissions for new node "capi20"

>                         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 == '>')

rename(device_name, alias) - rename("capi20", "capi20.20") - rename
    the device to its intendet name "capi20.20"

>                                         symlink(alias, device_name);
>                                 free(alias);
>                         }

So you can see the sequence mentioned in my first mail:

    unlink capi20
    mknod  capi20
       chown
       chmod
    rename capi20 to capi20.20
       symlink

The patch attached to the followup on my first mail should prevent
this by directly creating the alias node instead of using an
unlink/mknod/rename sequence.

regards,
Jean