[PATCH RESEND] mdev: fix sysfs traversal with CONFIG_SYSFS_DEPRECATED_V2=y

[Gregory Fong]

On Mon, Aug 3, 2015 at 9:07 AM, Denys Vlasenko <vda.linux at googlemail.com> wrote:
> On Wed, Jul 15, 2015 at 11:10 PM, Gregory Fong <gregory.0xf0 at gmail.com> wrote:
>> From: Simon Edlund <simon at edlund.nl>
>>
>> When mdev -s traverses the /sys directory looking for "dev" files, it
>> starts with the block devices under /sys/block, and will find the "dev"
>> file through the symlink, and create a block device node.  In the next
>> stage it will scan the /sys/class looking for char devices, and will
>> find mtdX/dev again, but this time the mknod will fail because there is
>> already a device node with that name.
>
> By swapping  recursive_action("/sys/class") and
> recursive_action("/sys/block"), now the same thing will happen
> when the second scan is done.
>
> Why this is better?

This is better because if you're using a newer kernel with both
CONFIG_SYSFS_DEPRECATED=y (and CONFIG_SYSFS_DEPRECATED_V2=y to enable
it), then where traversing /sys/block with the deprecated layout:
- /sys/block/mtdblock0 is not a symlink, but an actual directory
- /sys/block/mtdblock0/device is a symlink to mtd0

IOW, on deprecated sysfs:
# ls -l /sys/block/mtdblock0
-r--r--r--    1 root     root          4096 Jan  1 00:00 alignment_offset
lrwxrwxrwx    1 root     root             0 Jan  1 00:00 bdi ->
../../devices/virtual/bdi/31:0
-r--r--r--    1 root     root          4096 Jan  1 00:00 capability
-r--r--r--    1 root     root          4096 Jan  1 00:00 dev
lrwxrwxrwx    1 root     root             0 Jan  1 00:00 device ->
../../devices/platform/brcmnand.0/mtd/mtd0
-r--r--r--    1 root     root          4096 Jan  1 00:00 discard_alignment
-r--r--r--    1 root     root          4096 Jan  1 00:00 ext_range
drwxr-xr-x    2 root     root             0 Jan  1 00:00 holders
-r--r--r--    1 root     root          4096 Jan  1 00:00 inflight
drwxr-xr-x    2 root     root             0 Jan  1 00:00 power
drwxr-xr-x    3 root     root             0 Jan  1 00:00 queue
-r--r--r--    1 root     root          4096 Jan  1 00:00 range
-r--r--r--    1 root     root          4096 Jan  1 00:00 removable
-r--r--r--    1 root     root          4096 Jan  1 00:00 ro
-r--r--r--    1 root     root          4096 Jan  1 00:00 size
drwxr-xr-x    2 root     root             0 Jan  1 00:00 slaves
-r--r--r--    1 root     root          4096 Jan  1 00:00 stat
lrwxrwxrwx    1 root     root             0 Jan  1 00:00 subsystem ->
../../block
-rw-r--r--    1 root     root          4096 Jan  1 00:00 uevent

On non-deprecated sysfs:
# ls -l /sys/block/mtdblock0
lrwxrwxrwx    1 root     root             0 Jan  3 18:57
/sys/block/mtdblock0 ->
../devices/platform/rdb/f03e2800.nand/mtd/mtd0/mtdblock0



Now, I hadn't really investigated the problem carefully enough until
now.  This patch simply solved a real problem for us when using
CONFIG_SYSFS_DEPRECATED, and I figured that submitting it would be at
least a good starting point for someone who actually understands this
better to figure out a better fix.

But let's see... since recursive_action is called with
ACTION_FOLLOWLINKS, it will end up checking both
/sys/block/mtdblock0/dev and /sys/block/mtdblock0/device/dev and
creating block devices for both of those.  That's not correct.  So I
think I now have a real fix here, which is to change the
recursive_action call that will only be invoked if
CONFIG_SYSFS_DEPRECATED=y.  In that case, we _shouldn't_ need to follow
symlinks because everything in the top level of /sys/block/ will be a
real directory, and following them will result in the aforementioned problem.

It was a bit surprising to me that this problem isn't also seen with
/sys/class/block, since that has the same hierarchy where it could
follow the link and get to /sys/class/block/mtdblock0/device/dev, but
looking at recursive_action(), it appears that links are only followed
where depth == 0, which would explain that.

I'm stuck using the gmail interface right now and so can't copy and
paste the new patch inline... will send to the list shortly.

Best regards,
Gregory

>
>> [gregory: added commit message to patch from BZ #6806]
>> Signed-off-by: Gregory Fong <gregory.0xf0 at gmail.com>
>> ---
>>  util-linux/mdev.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/util-linux/mdev.c b/util-linux/mdev.c
>> index ca4b915..4495b0a 100644
>> --- a/util-linux/mdev.c
>> +++ b/util-linux/mdev.c
>> @@ -1063,6 +1063,9 @@ int mdev_main(int argc UNUSED_PARAM, char **argv)
>>
>>                 putenv((char*)"ACTION=add");
>>
>> +               recursive_action("/sys/class",
>> +                       ACTION_RECURSE | ACTION_FOLLOWLINKS,
>> +                       fileAction, dirAction, temp, 0);
>>                 /* ACTION_FOLLOWLINKS is needed since in newer kernels
>>                  * /sys/block/loop* (for example) are symlinks to dirs,
>>                  * not real directories.
>> @@ -1079,9 +1082,6 @@ int mdev_main(int argc UNUSED_PARAM, char **argv)
>>                                 ACTION_RECURSE | ACTION_FOLLOWLINKS | ACTION_QUIET,
>>                                 fileAction, dirAction, temp, 0);
>>                 }
>> -               recursive_action("/sys/class",
>> -                       ACTION_RECURSE | ACTION_FOLLOWLINKS,
>> -                       fileAction, dirAction, temp, 0);
>>         } else {
>>                 char *fw;
>>                 char *seq;
>> --
>> 1.9.1
>>
>> _______________________________________________
>> busybox mailing list
>> busybox at busybox.net
>> http://lists.busybox.net/mailman/listinfo/busybox