You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
If multiple block devices paths share the same prefix e.g. /dev/mmcblk0p1 and /dev/mmcblk0p10 the commands block info confuses them.
In my case /dev/mmcblk0p1 is a boot partition and /dev/mmcblk0p10 is a data partition.
There are a bunch of other partitions, which I'll omit for the sake clarity in the following example.
After boot the output of mount looks like this:
root@foobar:~# mount
/dev/root on /rom type squashfs (ro,relatime)
proc on /proc type proc (rw,nosuid,nodev,noexec,noatime)
sysfs on /sys type sysfs (rw,nosuid,nodev,noexec,noatime)
tmpfs on /tmp type tmpfs (rw,nosuid,nodev,noatime)
tmpfs on /dev type tmpfs (rw,nosuid,relatime,size=512k,mode=755)
devpts on /dev/pts type devpts (rw,nosuid,noexec,relatime,mode=600,ptmxmode=000)
debugfs on /sys/kernel/debug type debugfs (rw,noatime)
/dev/mmcblk0p10 on /data type ext4 (rw,relatime,data=ordered)
/dev/mmcblk0p1 on /uboot type vfat (rw,sync,relatime,fmask=0000,dmask=0000,allow_utime=0022,codepage=437,iocharset=iso8859-1,shortname=mixed,errors=remount-ro)
...
At least everything is mounted in the right place.
If I do a block info I get:
root@foobar:~# block info
/dev/mmcblk0p1: UUID="0037-0032" LABEL="" VERSION="FAT16" MOUNT="/data" TYPE="vfat"
/dev/mmcblk0p10: UUID="d8dff841-edc4-4094-ae68-f6f0596c2277" VERSION="1.0" MOUNT="/data" TYPE="ext4"
...
Which is a little bit odd, as /dev/mmcblk0p1 is clearly not mounted under /data.
I did some digging in the sources and found the culprit in [[https://git.openwrt.org/?p=project/fstools.git;a=blob;f=block.c;h=cfc87273ee9969a153a4beededc61a5c6ba40a16;hb=HEAD#l578|block.c:find_mount_point]].
In [[https://git.openwrt.org/?p=project/fstools.git;a=blob;f=block.c;h=cfc87273ee9969a153a4beededc61a5c6ba40a16;hb=HEAD#l646|line 646]] in block.c:
*pos = '\0';
devname = tmp;
if (!strncmp(block, devname, len)) { // <- problem
point = strdup(cpoint);
break;
}
block is the parameter passed to find_mount_point, len is the string length of block and devname is a block device name read from /proc/self/mountinfo.
If the string in block is a prefix of the string in devname, !strncmp will always be true as, therefore the mount point in this line will be returned to caller.
In my case the line for /dev/mmcblk0p10 is encountered first, therefore /data is returned as mountpoint for both /dev/mmcblk0p10 and /dev/mmcblk0p1.
Since the idea here is to look for an exactly equal string I would suggest to compare the string length as well as calling strncmp.
See the attached patch for my temporary solution.
As a last remark: I could not determine if this affects other functionalities of /sbin/block as well,
but since find_mount_point is also called during umounting this might be more than just a display bug.
The text was updated successfully, but these errors were encountered:
I didn't simply replace all strncmp() calls with strcmp() ones but analyzed the surrounding code context to see whether it makes sense or not.
For the changed places where strncmp() was used, it was not useful, inappropriate or offered no advantage over a simple strcmp().
''strncmp(block, devname, len)'' (with ''len = strlen(block)'') wrongly matches substrings since it does not compare trailing ''\0''
''strncmp(block, devname, len + 1)'' (with ''len = strlen(block)'') is identical to ''strcmp(block, devname)'' but more expensive due to the required ''strlen()'' call
''strncmp(name, volname, strlen(volname) + 1)'' is equivalent to ''strcmp(name, volname)''
I would agree that changing ''strncmp()'' to ''strcmp()'' is a problem when the length argument is not derived from ''strlen()'' but for the cases that have been changed I do not see the problem.
What would have been the best choice in your opinion?
Looks like jow- misunderstood LongHairedHacker, the later was saying that the changes from strncmp to strcmp were a good thing. Regardless, it appears as though this bug has been fixed, though I haven't tested it. I suggest this issue be closed.
LongHairedHacker:
I've encountered a bug in /sbin/block.
If multiple block devices paths share the same prefix e.g. /dev/mmcblk0p1 and /dev/mmcblk0p10 the commands block info confuses them.
In my case /dev/mmcblk0p1 is a boot partition and /dev/mmcblk0p10 is a data partition.
There are a bunch of other partitions, which I'll omit for the sake clarity in the following example.
/etc/config/fstab:
config 'global'
option anon_swap '0'
option anon_mount '0'
option auto_swap '1'
option auto_mount '1'
option delay_root '5'
option check_fs '0'
config 'mount'
option target '/data'
option device '/dev/mmcblk0p10'
option fstype 'ext4'
option enabled '1'
config 'mount'
option device '/dev/mmcblk0p1'
option target '/uboot'
option fstype 'vfat'
option options 'rw,sync'
option enabled '1'
...
After boot the output of mount looks like this:
root@foobar:~# mount
/dev/root on /rom type squashfs (ro,relatime)
proc on /proc type proc (rw,nosuid,nodev,noexec,noatime)
sysfs on /sys type sysfs (rw,nosuid,nodev,noexec,noatime)
tmpfs on /tmp type tmpfs (rw,nosuid,nodev,noatime)
tmpfs on /dev type tmpfs (rw,nosuid,relatime,size=512k,mode=755)
devpts on /dev/pts type devpts (rw,nosuid,noexec,relatime,mode=600,ptmxmode=000)
debugfs on /sys/kernel/debug type debugfs (rw,noatime)
/dev/mmcblk0p10 on /data type ext4 (rw,relatime,data=ordered)
/dev/mmcblk0p1 on /uboot type vfat (rw,sync,relatime,fmask=0000,dmask=0000,allow_utime=0022,codepage=437,iocharset=iso8859-1,shortname=mixed,errors=remount-ro)
...
At least everything is mounted in the right place.
If I do a block info I get:
root@foobar:~# block info
/dev/mmcblk0p1: UUID="0037-0032" LABEL="" VERSION="FAT16" MOUNT="/data" TYPE="vfat"
/dev/mmcblk0p10: UUID="d8dff841-edc4-4094-ae68-f6f0596c2277" VERSION="1.0" MOUNT="/data" TYPE="ext4"
...
Which is a little bit odd, as /dev/mmcblk0p1 is clearly not mounted under /data.
I did some digging in the sources and found the culprit in [[https://git.openwrt.org/?p=project/fstools.git;a=blob;f=block.c;h=cfc87273ee9969a153a4beededc61a5c6ba40a16;hb=HEAD#l578|block.c:find_mount_point]].
In [[https://git.openwrt.org/?p=project/fstools.git;a=blob;f=block.c;h=cfc87273ee9969a153a4beededc61a5c6ba40a16;hb=HEAD#l646|line 646]] in block.c:
*pos = '\0';
devname = tmp;
if (!strncmp(block, devname, len)) { // <- problem
point = strdup(cpoint);
break;
}
block is the parameter passed to find_mount_point, len is the string length of block and devname is a block device name read from /proc/self/mountinfo.
If the string in block is a prefix of the string in devname, !strncmp will always be true as, therefore the mount point in this line will be returned to caller.
In my case the line for /dev/mmcblk0p10 is encountered first, therefore /data is returned as mountpoint for both /dev/mmcblk0p10 and /dev/mmcblk0p1.
Since the idea here is to look for an exactly equal string I would suggest to compare the string length as well as calling strncmp.
See the attached patch for my temporary solution.
As a last remark: I could not determine if this affects other functionalities of /sbin/block as well,
but since find_mount_point is also called during umounting this might be more than just a display bug.
The text was updated successfully, but these errors were encountered: