bad handling of unpartitioned device in sysfs_devno_to_wholedisk() (which breaks mkfs.btrfs)

bad handling of unpartitioned device in sysfs_devno_to_wholedisk() (which breaks mkfs.btrfs)

[Tom Yan]

So I noticed that SSD detection does work on unpartitioned devices in
mkfs.btrfs somehow:
https://bugzilla.kernel.org/show_bug.cgi?id=102921

Later I found out that it breaks at blkid_devno_to_wholedisk() in is_ssd():
http://git.kernel.org/cgit/linux/kernel/git/kdave/btrfs-progs.git/tree/mkfs.c?h=v4.2.3#n1103

which Elliot had shown an example with strace:
https://lists.01.org/pipermail/linux-nvdimm/2015-September/002109.html

And I think the problem occurs in the sysfs_get_devname() here:
https://git.kernel.org/cgit/utils/util-linux/util-linux.git/tree/lib/sysfs.c?h=v2.27#n785

Since sysfs_get_devname() has to call sysfs_readlink() later, which
output a long full device path in /sys, I don't think we should call
it directly with the buffer "diskname", which people won't expect that
it has to be large enough to carry the path in the middle of the
process. For example in is_sdd(), a char array of size 32 is used
("wholedisk").

Re: bad handling of unpartitioned device in sysfs_devno_to_wholedisk() (which breaks mkfs.btrfs)

[Karel Zak]

On Fri, Oct 30, 2015 at 09:43:28AM +0800, Tom Yan wrote:
> So I noticed that SSD detection does work on unpartitioned devices in
> mkfs.btrfs somehow:
> https://bugzilla.kernel.org/show_bug.cgi?id=102921
> 
> Later I found out that it breaks at blkid_devno_to_wholedisk() in is_ssd():
> http://git.kernel.org/cgit/linux/kernel/git/kdave/btrfs-progs.git/tree/mkfs.c?h=v4.2.3#n1103
> 
> which Elliot had shown an example with strace:
> https://lists.01.org/pipermail/linux-nvdimm/2015-September/002109.html
> 
> And I think the problem occurs in the sysfs_get_devname() here:
> https://git.kernel.org/cgit/utils/util-linux/util-linux.git/tree/lib/sysfs.c?h=v2.27#n785
> 
> Since sysfs_get_devname() has to call sysfs_readlink() later, which
> output a long full device path in /sys, I don't think we should call
> it directly with the buffer "diskname", which people won't expect that
> it has to be large enough to carry the path in the middle of the
> process. For example in is_sdd(), a char array of size 32 is used
> ("wholedisk").

You're right. The function sysfs_get_devname() is not too elegant as
it uses devname buffer for readlink. Fixed, the bugfix will be in
v2.27.1.

Thanks!

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com