Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

0.6.5.9 regression (compared to 0.6.5.8 and previous) : loop mounted luks encrypted files on zfs are read-only #5776

Closed
truatpasteurdotfr opened this issue Feb 10, 2017 · 6 comments
Closed

0.6.5.9 regression (compared to 0.6.5.8 and previous) : loop mounted luks encrypted files on zfs are read-only #5776

truatpasteurdotfr opened this issue Feb 10, 2017 · 6 comments
Labels
Type: Regression Indicates a functional regression

Comments

@truatpasteurdotfr
Copy link

System information

Type Version/Name
Distribution Name CentOS
Distribution Version 7.3
Linux Kernel 3.10.0-514.2.2.el7.x86_64 and 3.10.0-514.6.1.el7.x86_64
Architecture x86_64
ZFS Version 0.6.5.9
SPL Version 0.6.5.9

Describe the problem you're observing

I have luks encrypted files in my zfs filesystems.
Since 0.6.5.9, when I loop mount them, the filesystem are read-only.

Describe how to reproduce the problem

boot a centos7 machine, install0.6.5.9 zfs (kmod-zfs) and create a file based luks file (xfs formatted), put some files inside.
Create a zpool, put the newly created luks file inside and mount it. It will be mounted read-only.

Doing the same on a 0.6.5.8 zfs, works fine, the luks file is mounted read-write.

Include any warning/errors/backtraces from the system logs
@truatpasteurdotfr
Copy link
Author

Here is my reproducible test case:
vagrant init centos/7
vagrant up
vagrant rsync
vagrant ssh
inside the vm
cd /vagrant and run the scripts in order
00-install-zfs-0.6.5.8.sh or 00-install-zfs-0.6.5.9.sh
10-create-luks.sh
20-mount-luks-ext4.sh
30-populate-luks-ext4.sh
40-umount-luks-ext4.sh
50-create-zfs.sh
60-populate-zfs.sh
70-mount-luks-zfs.sh
80-umount-luks-zfs.sh

5776.zip

@behlendorf behlendorf added the Type: Regression Indicates a functional regression label Feb 10, 2017
@dfranganillo
Copy link

Yeah, We've been hit pretty hard with this regresion. Any workaround whatsoever?

@behlendorf
Copy link
Contributor

It would be helpful if someone with this configuration already set up could git bisect the 0.6.5.8 to 0.6.5.9 changes to determine which patch introduced the issue.

@loli10K
Copy link
Contributor

loli10K commented Mar 2, 2017

I think we can take LUKS out of the loop (no pun intended), this is reproducible with any "loop" device:

[root@centos ~]# df -t zfs /$POOLNAME
Filesystem     1K-blocks  Used Available Use% Mounted on
testpool           81792   512     81280   1% /testpool
[root@centos ~]# losetup -f /$POOLNAME/image.dat 
losetup: /testpool/image.dat: warning: file does not fit into a 512-byte sector the end of the file will be ignored.
[root@centos ~]# cat /sys/block/loop0/ro
1
[root@centos ~]#

A quick git bisect shows that the issue was introduced in 933ec99 (Retire .write/.read file operations).

I used SystemTap to print file->f_op when we are in the loop device setup code (http://lxr.free-electrons.com/source/drivers/block/loop.c?v=3.10#L868).

[root@centos ~]# stap -d loop -d spl -d zfs -d kernel    -g -v    -DMAXSTRINGLEN=1024    -e '
probe
kernel.function("fget").return
{
   if (probefunc() == "lo_ioctl") {
      printf("%s\n", probefunc());
      printf("%s\n", $return->f_op$$);
      print_backtrace();
   }
}'
Pass 1: parsed user script and 118 library scripts using 222864virt/35720res/3284shr/32900data kb, in 310usr/10sys/326real ms.
Pass 2: analyzed script: 1 probe, 36 functions, 4 embeds, 0 globals using 261216virt/75288res/4508shr/71252data kb, in 630usr/70sys/697real ms.
Pass 3: using cached /root/.systemtap/cache/84/stap_84a9cff4d5031aea88de11c9824b2c8c_33587.c
Pass 4: using cached /root/.systemtap/cache/84/stap_84a9cff4d5031aea88de11c9824b2c8c_33587.ko
Pass 5: starting run.
lo_ioctl
{.owner=0x0, .llseek=0xffffffffa07df640, .read=0x0, .write=0x0, .aio_read=0xffffffffa07dfc80, .aio_write=0xffffffffa07e01c0, .readdir=0x0, .poll=0x0, .unlocked_ioctl=0xffffffffa07dec30, .compat_ioctl=0xffffffffa07ded10, .mmap=0xffffffffa07deea0, .open=0xffffffffa07df3d0, .flush=0x0, .release=0xffffffffa07df160, .fsync=0xffffffffa07df4f0, .aio_fsync=0xffffffffa07df610, .fasync=0x0, .lock=0x0, .sendpage=0x0, .get_unmapped_area=0x0, .check_flags=0x0, .flock=0x0, .splice_write=0x0, .splice_read=0x0, <union>={.setlease=0x0, .__UNIQUE_ID_rh_kabi_hide11={.setlease=0x0}, <union>={}}, .fallocate=0xffffffffa07e0460, .show_fdinfo=0x0}
Returning from:  0xffffffff8121bfe0 : fget+0x0/0x90 [kernel]
Returning to  :  0xffffffffa047cf2c : lo_ioctl+0xec/0x730 [loop]
 0xffffffff812fd1b0 : blkdev_ioctl+0x270/0x980 [kernel]
 0xffffffff81239f71 : block_ioctl+0x41/0x50 [kernel]
 0xffffffff81212025 : do_vfs_ioctl+0x2d5/0x4b0 [kernel]
 0xffffffff812122a1 : SyS_ioctl+0xa1/0xc0 [kernel]
 0xffffffff81696ad2 : tracesys+0xdd/0xe2 [kernel]

The code for loop_set_fd() shows that if file->f_op->write is NULL we set the device RO:

868         if (!(file->f_mode & FMODE_WRITE) || !(mode & FMODE_WRITE) ||
869             !file->f_op->write)
870                 lo_flags |= LO_FLAGS_READ_ONLY;

@tuxoko tuxoko mentioned this issue Mar 2, 2017
Merged
11 tasks
@behlendorf
Copy link
Contributor

@loli10K thanks for bisecting this. It appears this check wasn't updated to check .write_iter until the 4.0 kernel, torvalds/linux@283e7e5.

@tuxoko
Copy link
Contributor

tuxoko commented Mar 2, 2017

Should be fixed in #5855, please give it a try.

l1k pushed a commit to l1k/zfs that referenced this issue Apr 17, 2017
@tuxoko @l1k
Fix loop device becomes read-only
79e2927 
Commit 933ec99 removes read and write from f_op because the vfs layer will
select iter_write or aio_write automatically. However, for Linux <= 4.0,
loop_set_fd will actually check f_op->write and set read-only if not exists.
This patch add them back and use the generic do_sync_{read,write} for
aio_{read,write} and new_sync_{read,write} for {read,write}_iter.

Reviewed-by: George Melikov <mail@gmelikov.ru>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Chunwei Chen <david.chen@osnexus.com>
Closes openzfs#5776
Closes openzfs#5855
tonyhutter pushed a commit to tonyhutter/zfs that referenced this issue May 18, 2017
@tuxoko @tonyhutter
Fix loop device becomes read-only
ef1ac6d 
Commit 933ec99 removes read and write from f_op because the vfs layer will
select iter_write or aio_write automatically. However, for Linux <= 4.0,
loop_set_fd will actually check f_op->write and set read-only if not exists.
This patch add them back and use the generic do_sync_{read,write} for
aio_{read,write} and new_sync_{read,write} for {read,write}_iter.

Reviewed-by: George Melikov <mail@gmelikov.ru>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Chunwei Chen <david.chen@osnexus.com>
Closes openzfs#5776
Closes openzfs#5855
tonyhutter pushed a commit that referenced this issue Jun 9, 2017
@tuxoko @tonyhutter
Fix loop device becomes read-only
2094a93 
Commit 933ec99 removes read and write from f_op because the vfs layer will
select iter_write or aio_write automatically. However, for Linux <= 4.0,
loop_set_fd will actually check f_op->write and set read-only if not exists.
This patch add them back and use the generic do_sync_{read,write} for
aio_{read,write} and new_sync_{read,write} for {read,write}_iter.

Reviewed-by: George Melikov <mail@gmelikov.ru>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Chunwei Chen <david.chen@osnexus.com>
Closes #5776
Closes #5855
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Type: Regression Indicates a functional regression
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@tuxoko @behlendorf @dfranganillo @loli10K @truatpasteurdotfr