[prev in list] [next in list] [prev in thread] [next in thread] 

List:       busybox
Subject:    RE: [PATCH] mdev: don't follow deprecated sysfs /sys/block symlinks
From:       "Mike McTernan (wavemobile)" <mike.mcternan () wavemobile ! com>
Date:       2015-09-25 11:19:21
Message-ID: B7D827E17A958449871286AB5AE2391A023FBE35 () GRID ! cxn ! local
[Download RAW message or body]

Hi Gregory,

I tested your patch and reviewed it.  It looks correct to me.

Here is some reasoning which is partly for my own benefit, but may be of use to \
others following.

> Additionally, following symlinks causes a bug for mtd in this case, e.g. several 
> people have been seeing that /dev/mtd0 was getting created by this traversal
> as a block device.  

Me too.

> Subsequent investigation revealed that was caused by this pass following the
> /sys/block/mtdblock0/device symlink and resulting in both
> - /sys/block/mtdblock0/dev
> - /sys/block/mtdblock0/device/dev
> being used here, and the second results in that incorrect /dev/mtd0.

Yes.  Here's the example on my system:

$ cat /sys/block/mtdblock0/dev                                                        \
 31:0                                                                                 \
 $ cat /sys/block/mtdblock0/device/dev                                                \
 90:0                                                                                 \
 $ realpath /sys/block/mtdblock0/dev                                                  \
                
/sys/block/mtdblock0/dev                                                              \
 $ realpath /sys/block/mtdblock0/device/dev                                           \
                
/sys/devices/platform/mxc_nand/mtd/mtd0/dev      

It looks like following symlinks from /sys/block isn't right (and your patch is \
correct) because in make_device() we see the following check:

        if (strstr(path, "/block/") || (G.subsystem && strncmp(G.subsystem, "block", \
5) == 0))  type = S_IFBLK;                               

Prior to that we have dirAction() which sets G.subsystem during sysfs traversal, but \
only when at depth 1:

static int FAST_FUNC dirAction, ...
                int depth)
{
        /* Extract device subsystem -- the name of the directory
         * under /sys/class/ */
        if (1 == depth) {
                free(G.subsystem);
...
                G.subsystem = strrchr(fileName, '/');
                if (G.subsystem) {
                        G.subsystem = xstrdup(G.subsystem + 1);
...
                }
        }

        return (depth >= MAX_SYSFS_DEPTH ? SKIP : TRUE);
}

So without using realpath() or other checks to determine the device type, following \
symlinks during /sys/block creation isn't correct as G.subsystem may then be wrongly \
set to "block".

> Change to not follow symlinks while traversing /sys/block to fix this.

In summary, I have tested this part of the patch, and it works for me.

The last part of the test was to check what happens with /sys/block/loopX.  In my \
system config they are real dirs, so the loopX devices correctly get made as block \
devices.  I suspect this is therefore okay.

Kind Regards,

Mike
_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox


[prev in list] [next in list] [prev in thread] [next in thread]