Comment 18 for bug 1829563

Revision history for this message
Mauricio Faria de Oliveira (mfo) wrote :

I/O Error Test 7
================

commit: "bcache: fix ioctl in flash device"

This test needs QEMU+GDB since there's no way to set the variable
from the kernel, because it references incorrect memory/pointer.

In the original kernel, the ioctl() to a flash-only volume
goes from ioctl_dev() down to flash_dev_ioctl()
(which always return -ENOTTY always) only if its backing device
has io_disable == 0, but gets -EIO if io_disable == 1
(which are both wrong as flash device has no backing device).

In the modified kernel, it always go down to flash_dev_ioctl(),
thus -ENOTTY in both cases.

Original kernel:

$ uname -rv
4.15.0-51-generic #55-Ubuntu SMP Wed May 15 14:27:21 UTC 2019

io_disable = 0:

# ./ioctl
ioctl: Inappropriate ioctl for device

io_disable = 1:

# ./ioctl
ioctl: Input/output error

Modified kernel:

# uname -rv
4.18.0-23-generic #24+test20190619b1 SMP Wed Jun 19 22:33:22 UTC 2019

io_disable = 0:

# ./ioctl
ioctl: Inappropriate ioctl for device

io_disable = 1:

# ./ioctl
ioctl: Inappropriate ioctl for device

Steps
-----

$ sudo -i
# ./setup.sh >/dev/null 2>&1
[ 57.607391] bcache: register_bdev() registered backing device dm-0
[ 57.611885] bcache: run_cache_set() invalidating existing data
[ 57.625712] bcache: register_cache() registered cache device dm-1
[ 58.605153] bcache: bch_cached_dev_attach() Caching dm-0 as bcache0 on set 08f1756b-081f-4545-a947-927d429f7ef1

# lsblk -e 252
NAME MAJ:MIN RM SIZE RO TYPE MOUNTPOINT
loop0 7:0 0 1G 0 loop
└─fake-loop0 253:0 0 1024M 0 dm
  └─bcache0 251:0 0 1024M 0 disk
loop1 7:1 0 1G 0 loop
└─fake-loop1 253:1 0 1024M 0 dm
  └─bcache0 251:0 0 1024M 0 disk

# echo $((128*1024**2)) > /sys/block/dm-1/bcache/set/flash_vol_create

# lsblk -e 252
NAME MAJ:MIN RM SIZE RO TYPE MOUNTPOINT
loop0 7:0 0 1G 0 loop
└─fake-loop0 253:0 0 1024M 0 dm
  └─bcache0 251:0 0 1024M 0 disk
loop1 7:1 0 1G 0 loop
└─fake-loop1 253:1 0 1024M 0 dm
  ├─bcache0 251:0 0 1024M 0 disk
  └─bcache1 251:128 0 128M 0 disk

# ls -1d /sys/block/bcache1/bcache/cache/volume*
/sys/block/bcache1/bcache/cache/volume1

# cat /sys/module/bcache/sections/.text
0xffffffffc05f4000

# cat ioctl.c
#include <sys/ioctl.h>
#include <fcntl.h>
#include <stdio.h>
#include <unistd.h>

int main() {
        int fd, rc;
        fd = open("/dev/bcache1", O_RDWR);
        if (fd == -1) {
                perror("open");
                return 0;
        }
        rc = ioctl(fd, 0xcc00ffee); // our cookie.
        if (rc == -1)
                perror("ioctl");
        close(fd);
        return 0;
}

# gcc -o ioctl ioctl.c
# ./ioctl

(gdb) add-symbol-file ~/ddeb-4.18.0-23-generic/usr/lib/debug/lib/modules/4.18.0-23-generic/kernel/drivers/md/bcache/bcache.ko 0xffffffffc05f4000

(gdb) b *ioctl_dev +0x13
(gdb) c

Thread 4 hit Breakpoint 1, ioctl_dev (b=<optimized out>, mode=134610975, cmd=3422617582, arg=2) at /build/linux-JHFGaE/linux-4.18.0/drivers/md/bcache/super.c:641
...

(gdb) p/x 3422617582
$53 = 0xcc00ffee // our cookie.

(gdb) print d.name
$54 = "volume1\000\000\000\000" // matches the volume above.

See it's an incorrect pointer.
No way to set it via any io_disable in sysfs.

(gdb) print ((struct cached_dev *)((void *)d - 0x10)).backing_dev_name
$75 = "\017\000\000\000\260\373\212s\277\230\377\377\260\373\212s\277\230\377\377 nԇ\377\377\377\377\060qh\211"

(gdb) print ((struct cached_dev *)((void *)d - 0x10)).io_disable
$76 = 0

With the default value (io_disable = 0):

# ./ioctl
ioctl: Inappropriate ioctl for device

(gdb) c

... repeat.

With the forced value (io_disable = 1):

(gdb) set ((struct cached_dev *)((void *)d - 0x10)).io_disable = 1
(gdb) c

# ./ioctl
ioctl: Input/output error