Hi Amir
Commit d8514d8edb5b ("ovl: copy up regular file using O_TMPFILE")
breaks squashfs with an ubifs overlay (both ubi volumes of the same
container).
Renaming a file results in an error "UBIFS error (ubi0:1 pid 1394):
ubifs_add_orphan: orphaned twice". This corrupts the the filesystem and
the next attempt to mount the overlay will fail.
Regards
Ralph
Bisect log:
git bisect start
# bad: [c02ed2e75ef4c74e41e421acb4ef1494671585e8] Linux 4.11-rc4
git bisect bad c02ed2e75ef4c74e41e421acb4ef1494671585e8
# good: [c470abd4fde40ea6a0846a2beab642a578c0b8cd] Linux 4.10
git bisect good c470abd4fde40ea6a0846a2beab642a578c0b8cd
# good: [bc49a7831b1137ce1c2dda1c57e3631655f5d2ae] Merge branch 'akpm' (patches from Andrew)
git bisect good bc49a7831b1137ce1c2dda1c57e3631655f5d2ae
# good: [738bc38d49e017fe7acb3596712518e22c225816] kernel/ksysfs.c: add __ro_after_init to bin_attribute structure
git bisect good 738bc38d49e017fe7acb3596712518e22c225816
# good: [3f80dd67c367878aaad16e458eebc3c8980bb841] Merge tag 'acpi-extra-4.11-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm
git bisect good 3f80dd67c367878aaad16e458eebc3c8980bb841
# bad: [4495c08e84729385774601b5146d51d9e5849f81] Linux 4.11-rc2
git bisect bad 4495c08e84729385774601b5146d51d9e5849f81
# bad: [2d62e0768d3c28536d4cfe4c40ba1e5e8e442a93] Merge tag 'kvm-4.11-2' of git://git.kernel.org/pub/scm/virt/kvm/kvm
git bisect bad 2d62e0768d3c28536d4cfe4c40ba1e5e8e442a93
# good: [1827adb11ad26b2290dc9fe2aaf54976b2439865] Merge branch 'WIP.sched-core-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
git bisect good 1827adb11ad26b2290dc9fe2aaf54976b2439865
# bad: [0b94da8dfc26ec2eb3e6640726e434abf8c53e49] Merge branch 'libnvdimm-fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/nvdimm/nvdimm
git bisect bad 0b94da8dfc26ec2eb3e6640726e434abf8c53e49
# bad: [0a040b2113ec226bcf56fcbe02d035bb5b30c35e] Merge branch 'for-next' of git://git.samba.org/sfrench/cifs-2.6
git bisect bad 0a040b2113ec226bcf56fcbe02d035bb5b30c35e
# good: [590dce2d4934fb909b112cd80c80486362337744] Merge branch 'rebased-statx' of git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs
git bisect good 590dce2d4934fb909b112cd80c80486362337744
# bad: [4e66c42c60fdf9be81837857454a41b39bf1b773] Merge branch 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/fuse
git bisect bad 4e66c42c60fdf9be81837857454a41b39bf1b773
# bad: [e593b2bf513dd4d3fbfa0f435392eea2c7f776f0] ovl: properly implement sync_filesystem()
git bisect bad e593b2bf513dd4d3fbfa0f435392eea2c7f776f0
# bad: [d8514d8edb5b045cf7f708e14f888ce760d60f0b] ovl: copy up regular file using O_TMPFILE
git bisect bad d8514d8edb5b045cf7f708e14f888ce760d60f0b
# good: [42f269b925405d9dd45014823e5057786d6ca452] ovl: rearrange code in ovl_copy_up_locked()
git bisect good 42f269b925405d9dd45014823e5057786d6ca452
# first bad commit: [d8514d8edb5b045cf7f708e14f888ce760d60f0b] ovl: copy up regular file using O_TMPFILE
On Tue, Mar 28, 2017 at 4:01 AM, Ralph Sennhauser
<[email protected]> wrote:
> Hi Amir
>
> Commit d8514d8edb5b ("ovl: copy up regular file using O_TMPFILE")
> breaks squashfs with an ubifs overlay (both ubi volumes of the same
> container).
>
Hi Ralph,
I am confused by the description above. Which are the 'both ubi volumes'?
Can you provide exact command of overlayfs mount, preferably
also a script to generate the lower/upper images and mount them
to remove any mkfs option doubts from test setup.
> Renaming a file results in an error "UBIFS error (ubi0:1 pid 1394):
> ubifs_add_orphan: orphaned twice". This corrupts the the filesystem and
> the next attempt to mount the overlay will fail.
>
Does that happen on any attempt to rename a file?
A file that was only is lower I suppose?
Can you provide a simple script with your test, setting up the lower/upper
files and triggering the bug.
Thanks,
Amir.
>
>
> Bisect log:
>
> git bisect start
> # bad: [c02ed2e75ef4c74e41e421acb4ef1494671585e8] Linux 4.11-rc4
> git bisect bad c02ed2e75ef4c74e41e421acb4ef1494671585e8
> # good: [c470abd4fde40ea6a0846a2beab642a578c0b8cd] Linux 4.10
> git bisect good c470abd4fde40ea6a0846a2beab642a578c0b8cd
> # good: [bc49a7831b1137ce1c2dda1c57e3631655f5d2ae] Merge branch 'akpm' (patches from Andrew)
> git bisect good bc49a7831b1137ce1c2dda1c57e3631655f5d2ae
> # good: [738bc38d49e017fe7acb3596712518e22c225816] kernel/ksysfs.c: add __ro_after_init to bin_attribute structure
> git bisect good 738bc38d49e017fe7acb3596712518e22c225816
> # good: [3f80dd67c367878aaad16e458eebc3c8980bb841] Merge tag 'acpi-extra-4.11-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm
> git bisect good 3f80dd67c367878aaad16e458eebc3c8980bb841
> # bad: [4495c08e84729385774601b5146d51d9e5849f81] Linux 4.11-rc2
> git bisect bad 4495c08e84729385774601b5146d51d9e5849f81
> # bad: [2d62e0768d3c28536d4cfe4c40ba1e5e8e442a93] Merge tag 'kvm-4.11-2' of git://git.kernel.org/pub/scm/virt/kvm/kvm
> git bisect bad 2d62e0768d3c28536d4cfe4c40ba1e5e8e442a93
> # good: [1827adb11ad26b2290dc9fe2aaf54976b2439865] Merge branch 'WIP.sched-core-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
> git bisect good 1827adb11ad26b2290dc9fe2aaf54976b2439865
> # bad: [0b94da8dfc26ec2eb3e6640726e434abf8c53e49] Merge branch 'libnvdimm-fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/nvdimm/nvdimm
> git bisect bad 0b94da8dfc26ec2eb3e6640726e434abf8c53e49
> # bad: [0a040b2113ec226bcf56fcbe02d035bb5b30c35e] Merge branch 'for-next' of git://git.samba.org/sfrench/cifs-2.6
> git bisect bad 0a040b2113ec226bcf56fcbe02d035bb5b30c35e
> # good: [590dce2d4934fb909b112cd80c80486362337744] Merge branch 'rebased-statx' of git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs
> git bisect good 590dce2d4934fb909b112cd80c80486362337744
> # bad: [4e66c42c60fdf9be81837857454a41b39bf1b773] Merge branch 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/fuse
> git bisect bad 4e66c42c60fdf9be81837857454a41b39bf1b773
> # bad: [e593b2bf513dd4d3fbfa0f435392eea2c7f776f0] ovl: properly implement sync_filesystem()
> git bisect bad e593b2bf513dd4d3fbfa0f435392eea2c7f776f0
> # bad: [d8514d8edb5b045cf7f708e14f888ce760d60f0b] ovl: copy up regular file using O_TMPFILE
> git bisect bad d8514d8edb5b045cf7f708e14f888ce760d60f0b
> # good: [42f269b925405d9dd45014823e5057786d6ca452] ovl: rearrange code in ovl_copy_up_locked()
> git bisect good 42f269b925405d9dd45014823e5057786d6ca452
> # first bad commit: [d8514d8edb5b045cf7f708e14f888ce760d60f0b] ovl: copy up regular file using O_TMPFILE
On Tue, Mar 28, 2017 at 5:27 AM, Amir Goldstein <[email protected]> wrote:
> On Tue, Mar 28, 2017 at 4:01 AM, Ralph Sennhauser
> <[email protected]> wrote:
>> Hi Amir
>>
>> Commit d8514d8edb5b ("ovl: copy up regular file using O_TMPFILE")
>> breaks squashfs with an ubifs overlay (both ubi volumes of the same
>> container).
>>
>
> Hi Ralph,
>
> I am confused by the description above. Which are the 'both ubi volumes'?
>
> Can you provide exact command of overlayfs mount, preferably
> also a script to generate the lower/upper images and mount them
> to remove any mkfs option doubts from test setup.
>
>> Renaming a file results in an error "UBIFS error (ubi0:1 pid 1394):
>> ubifs_add_orphan: orphaned twice". This corrupts the the filesystem and
>> the next attempt to mount the overlay will fail.
>>
Looking at the ubifs code, it does not appear that ubifs_link() ever calls
ubifs_delete_orphan() for the case of linking a temp file (nlink == 0),
so this looks like a ubifs bug.
Is ubifs being tested with xfstests? This should have been caught by test
generic/004 (link tempfile then delete it).
A quick fix for ubifs+overlayfs would be to disable ubifs O_TMPFILE
support (because it is broken) and then overlayfs won't try to copy up
using tmpfile.
Amir.
On Tue, 28 Mar 2017 05:27:03 -0400
Amir Goldstein <[email protected]> wrote:
> On Tue, Mar 28, 2017 at 4:01 AM, Ralph Sennhauser
> <[email protected]> wrote:
> > Hi Amir
> >
> > Commit d8514d8edb5b ("ovl: copy up regular file using O_TMPFILE")
> > breaks squashfs with an ubifs overlay (both ubi volumes of the same
> > container).
> >
>
> Hi Ralph,
>
> I am confused by the description above. Which are the 'both ubi
> volumes'?
The ubi container has two volumes, the first is a squashfs, the second
volume an ubifs. The latter is mounted as an overlay.
>
> Can you provide exact command of overlayfs mount, preferably
> also a script to generate the lower/upper images and mount them
> to remove any mkfs option doubts from test setup.
Both I mount from the initramfs as follows (rom / overlay are empty in
the initramfs):
mount -o rw,nosuid,nodev,noexec,noatime -t proc proc /proc || rescue_shell "proc"
mount -o rw,nosuid,nodev,noexec,noatime -t sysfs sysfs /sys || rescue_shell "sys"
mount -o rw,nosuid -t devtmpfs devtmpfs /dev || rescue_shell "dev"
ubiattach -m $(get_mtd_from_root_arg) /dev/ubi_ctrl || rescue_shell "attach"
ubiblock --create /dev/ubi0_0 || rescue_shell || "block"
mount -o ro -t squashfs /dev/ubiblock0_0 /rom || rescue_shell "mount rootfs"
mount -o rw,noatime -t ubifs /dev/ubi0_1 /overlay || rescue_shell "mount rootfs_data"
mkdir -p /overlay/upper || rescue_shell "mkdir upper"
mkdir -p /overlay/work || rescue_shell "mkdir work"
mount -o rw,noatime,lowerdir=/rom,upperdir=/overlay/upper,workdir=/overlay/work \
-t overlay overlay /newroot || rescue_shell "mount overlay"
mount --move /rom /newroot/rom || rescue_shell "move rootfs"
mount --move /overlay /newroot/overlay || rescue_shell "move rootfs_data"
mount --move /dev /newroot/dev || rescue_shell "move dev"
mount --move /sys /newroot/sys || rescue_shell "move sys"
mount --move /proc /newroot/proc || rescue_shell "move proc"
exec switch_root /newroot /sbin/init
I use OpenWrt as a basis, replacing the kernel with a vanilla one.
The options used to generate the file systems are:
Squashfs: -p 128KiB -m 2048 -s 512 -O 2048
Ubifs: -m 2048 -e 124KiB -c 4096 -F
>
> > Renaming a file results in an error "UBIFS error (ubi0:1 pid 1394):
> > ubifs_add_orphan: orphaned twice". This corrupts the the filesystem
> > and the next attempt to mount the overlay will fail.
> >
>
> Does that happen on any attempt to rename a file?
> A file that was only is lower I suppose?
That's how I trigger it, yes. Can reproduce it on any attempt.
> Can you provide a simple script with your test, setting up the
> lower/upper files and triggering the bug.
Any more you need than the above mount script? A call to "mv somefile
somefile.back && reboot" on a fresh install is all I do.
Thanks
Ralph
PS: Reverting 01ad3eb8a073 ("ovl: concurrent copy up of regular files")
as a dependency and the commit mentioned in Subject fix the issue for
me. Tested on v4.11-rc4 and next-20170327.
On Tue, Mar 28, 2017 at 6:45 AM, Ralph Sennhauser
<[email protected]> wrote:
> On Tue, 28 Mar 2017 05:27:03 -0400
> Amir Goldstein <[email protected]> wrote:
>
>> On Tue, Mar 28, 2017 at 4:01 AM, Ralph Sennhauser
>> <[email protected]> wrote:
>> > Hi Amir
>> >
>> > Commit d8514d8edb5b ("ovl: copy up regular file using O_TMPFILE")
>> > breaks squashfs with an ubifs overlay (both ubi volumes of the same
>> > container).
>> >
>>
>> Hi Ralph,
>>
>> I am confused by the description above. Which are the 'both ubi
>> volumes'?
>
> The ubi container has two volumes, the first is a squashfs, the second
> volume an ubifs. The latter is mounted as an overlay.
>
>>
>> Can you provide exact command of overlayfs mount, preferably
>> also a script to generate the lower/upper images and mount them
>> to remove any mkfs option doubts from test setup.
>
> Both I mount from the initramfs as follows (rom / overlay are empty in
> the initramfs):
>
> mount -o rw,nosuid,nodev,noexec,noatime -t proc proc /proc || rescue_shell "proc"
> mount -o rw,nosuid,nodev,noexec,noatime -t sysfs sysfs /sys || rescue_shell "sys"
> mount -o rw,nosuid -t devtmpfs devtmpfs /dev || rescue_shell "dev"
>
> ubiattach -m $(get_mtd_from_root_arg) /dev/ubi_ctrl || rescue_shell "attach"
> ubiblock --create /dev/ubi0_0 || rescue_shell || "block"
>
> mount -o ro -t squashfs /dev/ubiblock0_0 /rom || rescue_shell "mount rootfs"
> mount -o rw,noatime -t ubifs /dev/ubi0_1 /overlay || rescue_shell "mount rootfs_data"
>
> mkdir -p /overlay/upper || rescue_shell "mkdir upper"
> mkdir -p /overlay/work || rescue_shell "mkdir work"
>
> mount -o rw,noatime,lowerdir=/rom,upperdir=/overlay/upper,workdir=/overlay/work \
> -t overlay overlay /newroot || rescue_shell "mount overlay"
>
> mount --move /rom /newroot/rom || rescue_shell "move rootfs"
> mount --move /overlay /newroot/overlay || rescue_shell "move rootfs_data"
>
> mount --move /dev /newroot/dev || rescue_shell "move dev"
> mount --move /sys /newroot/sys || rescue_shell "move sys"
> mount --move /proc /newroot/proc || rescue_shell "move proc"
>
> exec switch_root /newroot /sbin/init
>
> I use OpenWrt as a basis, replacing the kernel with a vanilla one.
>
> The options used to generate the file systems are:
>
> Squashfs: -p 128KiB -m 2048 -s 512 -O 2048
> Ubifs: -m 2048 -e 124KiB -c 4096 -F
>
>>
>> > Renaming a file results in an error "UBIFS error (ubi0:1 pid 1394):
>> > ubifs_add_orphan: orphaned twice". This corrupts the the filesystem
>> > and the next attempt to mount the overlay will fail.
>> >
>>
>> Does that happen on any attempt to rename a file?
>> A file that was only is lower I suppose?
>
> That's how I trigger it, yes. Can reproduce it on any attempt.
>
>> Can you provide a simple script with your test, setting up the
>> lower/upper files and triggering the bug.
>
> Any more you need than the above mount script? A call to "mv somefile
> somefile.back && reboot" on a fresh install is all I do.
>
> Thanks
> Ralph
>
> PS: Reverting 01ad3eb8a073 ("ovl: concurrent copy up of regular files")
> as a dependency and the commit mentioned in Subject fix the issue for
> me. Tested on v4.11-rc4 and next-20170327.
That is not surprising.
Overlayfs now uses O_TMPFILE for copy up and it works fine with all the
file systems I tested (tmpfs, xfs, ext4).
If I am right and O_TMPFILE is broken in ubifs, you are most likely the first
person to test it (indirectly by overlayfs).
Please try to reproduce the bug with following patch to disable ubifs
O_TMPFILE support:
--- a/fs/ubifs/dir.c
+++ b/fs/ubifs/dir.c
@@ -1685,7 +1685,7 @@ const struct inode_operations
ubifs_dir_inode_operations = {
#ifdef CONFIG_UBIFS_ATIME_SUPPORT
.update_time = ubifs_update_time,
#endif
- .tmpfile = ubifs_tmpfile,
+ //.tmpfile = ubifs_tmpfile,
};
Hi Amir,
On Tue, 28 Mar 2017 07:03:11 -0400
Amir Goldstein <[email protected]> wrote:
> Overlayfs now uses O_TMPFILE for copy up and it works fine with all
> the file systems I tested (tmpfs, xfs, ext4).
> If I am right and O_TMPFILE is broken in ubifs, you are most likely
> the first person to test it (indirectly by overlayfs).
>
> Please try to reproduce the bug with following patch to disable ubifs
> O_TMPFILE support:
>
> --- a/fs/ubifs/dir.c
> +++ b/fs/ubifs/dir.c
> @@ -1685,7 +1685,7 @@ const struct inode_operations
> ubifs_dir_inode_operations = {
> #ifdef CONFIG_UBIFS_ATIME_SUPPORT
> .update_time = ubifs_update_time,
> #endif
> - .tmpfile = ubifs_tmpfile,
> + //.tmpfile = ubifs_tmpfile,
> };
Get a unused warning during build but all seems to be working fine now.
Thanks
Ralph
On Tue, Mar 28, 2017 at 7:28 AM, Ralph Sennhauser
<[email protected]> wrote:
> Hi Amir,
>
> On Tue, 28 Mar 2017 07:03:11 -0400
> Amir Goldstein <[email protected]> wrote:
>
>> Overlayfs now uses O_TMPFILE for copy up and it works fine with all
>> the file systems I tested (tmpfs, xfs, ext4).
>> If I am right and O_TMPFILE is broken in ubifs, you are most likely
>> the first person to test it (indirectly by overlayfs).
>>
>> Please try to reproduce the bug with following patch to disable ubifs
>> O_TMPFILE support:
>>
>> --- a/fs/ubifs/dir.c
>> +++ b/fs/ubifs/dir.c
>> @@ -1685,7 +1685,7 @@ const struct inode_operations
>> ubifs_dir_inode_operations = {
>> #ifdef CONFIG_UBIFS_ATIME_SUPPORT
>> .update_time = ubifs_update_time,
>> #endif
>> - .tmpfile = ubifs_tmpfile,
>> + //.tmpfile = ubifs_tmpfile,
>> };
>
> Get a unused warning during build but all seems to be working fine now.
>
OK. I'll wait for ubifs developers to fix the bug.
Otherwise, I'll send a proper patch to disable ubifs O_TMPFILE support.
Will add tested-by you.
Thanks!
On Tue, 28 Mar 2017 08:08:51 -0400
Amir Goldstein <[email protected]> wrote:
> On Tue, Mar 28, 2017 at 7:28 AM, Ralph Sennhauser
> <[email protected]> wrote:
> > Hi Amir,
> >
> > On Tue, 28 Mar 2017 07:03:11 -0400
> > Amir Goldstein <[email protected]> wrote:
> >
> >> Overlayfs now uses O_TMPFILE for copy up and it works fine with all
> >> the file systems I tested (tmpfs, xfs, ext4).
> >> If I am right and O_TMPFILE is broken in ubifs, you are most likely
> >> the first person to test it (indirectly by overlayfs).
> >>
> >> Please try to reproduce the bug with following patch to disable
> >> ubifs O_TMPFILE support:
> >>
> >> --- a/fs/ubifs/dir.c
> >> +++ b/fs/ubifs/dir.c
> >> @@ -1685,7 +1685,7 @@ const struct inode_operations
> >> ubifs_dir_inode_operations = {
> >> #ifdef CONFIG_UBIFS_ATIME_SUPPORT
> >> .update_time = ubifs_update_time,
> >> #endif
> >> - .tmpfile = ubifs_tmpfile,
> >> + //.tmpfile = ubifs_tmpfile,
> >> };
> >
> > Get a unused warning during build but all seems to be working fine
> > now.
>
> OK. I'll wait for ubifs developers to fix the bug.
> Otherwise, I'll send a proper patch to disable ubifs O_TMPFILE
> support. Will add tested-by you.
Sounds like a good plan, there is still time for 4.11-rc5. Fine with
you adding my tested-by in case it will come to this.
Appreciated
Ralph
>
> Thanks!
On Tue, Mar 28, 2017 at 3:16 PM, Ralph Sennhauser
<[email protected]> wrote:
> On Tue, 28 Mar 2017 08:08:51 -0400
> Amir Goldstein <[email protected]> wrote:
>
>> On Tue, Mar 28, 2017 at 7:28 AM, Ralph Sennhauser
>> <[email protected]> wrote:
>> > Hi Amir,
>> >
>> > On Tue, 28 Mar 2017 07:03:11 -0400
>> > Amir Goldstein <[email protected]> wrote:
>> >
>> >> Overlayfs now uses O_TMPFILE for copy up and it works fine with all
>> >> the file systems I tested (tmpfs, xfs, ext4).
>> >> If I am right and O_TMPFILE is broken in ubifs, you are most likely
>> >> the first person to test it (indirectly by overlayfs).
>> >>
>> >> Please try to reproduce the bug with following patch to disable
>> >> ubifs O_TMPFILE support:
>> >>
>> >> --- a/fs/ubifs/dir.c
>> >> +++ b/fs/ubifs/dir.c
>> >> @@ -1685,7 +1685,7 @@ const struct inode_operations
>> >> ubifs_dir_inode_operations = {
>> >> #ifdef CONFIG_UBIFS_ATIME_SUPPORT
>> >> .update_time = ubifs_update_time,
>> >> #endif
>> >> - .tmpfile = ubifs_tmpfile,
>> >> + //.tmpfile = ubifs_tmpfile,
>> >> };
>> >
>> > Get a unused warning during build but all seems to be working fine
>> > now.
>>
>> OK. I'll wait for ubifs developers to fix the bug.
>> Otherwise, I'll send a proper patch to disable ubifs O_TMPFILE
>> support. Will add tested-by you.
>
> Sounds like a good plan, there is still time for 4.11-rc5. Fine with
> you adding my tested-by in case it will come to this.
>
Ralph,
Can you please try to reproduce the problem with this command
on ubifs:
# create and link a tmpfile - then remove it
sudo rm -rf foo; sudo xfs_io -T -c "flink foo" . ; ls -l foo; sudo rm foo
That's the gist of xfstest generic/004.
You should expect the same behavior of corrupted fs after boot.
This issue should be reproduced since kernel 4.9 when ubifs tmpfile
support was added.
I hope this info can help the ubifs developers fix the problem
independently from overlayfs setup.
Thanks,
Amir.
Amir,
Am 28.03.2017 um 12:43 schrieb Amir Goldstein:
> On Tue, Mar 28, 2017 at 5:27 AM, Amir Goldstein <[email protected]> wrote:
>> On Tue, Mar 28, 2017 at 4:01 AM, Ralph Sennhauser
>> <[email protected]> wrote:
>>> Hi Amir
>>>
>>> Commit d8514d8edb5b ("ovl: copy up regular file using O_TMPFILE")
>>> breaks squashfs with an ubifs overlay (both ubi volumes of the same
>>> container).
>>>
>>
>> Hi Ralph,
>>
>> I am confused by the description above. Which are the 'both ubi volumes'?
>>
>> Can you provide exact command of overlayfs mount, preferably
>> also a script to generate the lower/upper images and mount them
>> to remove any mkfs option doubts from test setup.
>>
>>> Renaming a file results in an error "UBIFS error (ubi0:1 pid 1394):
>>> ubifs_add_orphan: orphaned twice". This corrupts the the filesystem and
>>> the next attempt to mount the overlay will fail.
>>>
>
> Looking at the ubifs code, it does not appear that ubifs_link() ever calls
> ubifs_delete_orphan() for the case of linking a temp file (nlink == 0),
> so this looks like a ubifs bug.
Thanks for the heads up, I'll look into this.
> Is ubifs being tested with xfstests? This should have been caught by test
> generic/004 (link tempfile then delete it).
Yes, it is.
David, can you please double-check wrt. test generic/004?
Thanks,
//richard
On Wed, 29 Mar 2017 22:16:10 +0300
Amir Goldstein <[email protected]> wrote:
> On Tue, Mar 28, 2017 at 3:16 PM, Ralph Sennhauser
> <[email protected]> wrote:
> > On Tue, 28 Mar 2017 08:08:51 -0400
> > Amir Goldstein <[email protected]> wrote:
> >
> >> On Tue, Mar 28, 2017 at 7:28 AM, Ralph Sennhauser
> >> <[email protected]> wrote:
> >> > Hi Amir,
> >> >
> >> > On Tue, 28 Mar 2017 07:03:11 -0400
> >> > Amir Goldstein <[email protected]> wrote:
> >> >
> >> >> Overlayfs now uses O_TMPFILE for copy up and it works fine with
> >> >> all the file systems I tested (tmpfs, xfs, ext4).
> >> >> If I am right and O_TMPFILE is broken in ubifs, you are most
> >> >> likely the first person to test it (indirectly by overlayfs).
> >> >>
> >> >> Please try to reproduce the bug with following patch to disable
> >> >> ubifs O_TMPFILE support:
> >> >>
> >> >> --- a/fs/ubifs/dir.c
> >> >> +++ b/fs/ubifs/dir.c
> >> >> @@ -1685,7 +1685,7 @@ const struct inode_operations
> >> >> ubifs_dir_inode_operations = {
> >> >> #ifdef CONFIG_UBIFS_ATIME_SUPPORT
> >> >> .update_time = ubifs_update_time,
> >> >> #endif
> >> >> - .tmpfile = ubifs_tmpfile,
> >> >> + //.tmpfile = ubifs_tmpfile,
> >> >> };
> >> >
> >> > Get a unused warning during build but all seems to be working
> >> > fine now.
> >>
> >> OK. I'll wait for ubifs developers to fix the bug.
> >> Otherwise, I'll send a proper patch to disable ubifs O_TMPFILE
> >> support. Will add tested-by you.
> >
> > Sounds like a good plan, there is still time for 4.11-rc5. Fine with
> > you adding my tested-by in case it will come to this.
> >
>
>
> Ralph,
>
> Can you please try to reproduce the problem with this command
> on ubifs:
Replaced the squashfs with the ubifs overlay with a plain ubifs root.
>
> # create and link a tmpfile - then remove it
> sudo rm -rf foo; sudo xfs_io -T -c "flink foo" . ; ls -l foo; sudo rm
> foo
next-20170328, obviously without your patch:
# xfs_io -T -c "flink foo" .
.: Not supported
# xfs_io -V
xfs_io version 4.9.0
# strace xfs_io -T -c "flink foo" .
execve("/sbin/xfs_io", ["xfs_io", "-T", "-c", "flink foo", "."], [/* 8 vars */]) = 0
set_tls(0xb6f17544, 0xbee0ac10, 0xb6f180a0, 0, 0xb6f1749c) = 0
set_tid_address(0xb6f174b8) = 2556
open("/etc/ld-musl-armhf.path", O_RDONLY|O_LARGEFILE|O_CLOEXEC) = -1 ENOENT (No such file or directory)
open("/lib/libgcc_s.so.1", O_RDONLY|O_LARGEFILE|O_CLOEXEC) = 3
fcntl64(3, F_SETFD, FD_CLOEXEC) = 0
fstat64(3, {st_mode=S_IFREG|0644, st_size=38203, ...}) = 0
read(3, "\177ELF\1\1\1\0\0\0\0\0\0\0\0\0\3\0(\0\1\0\0\0\240;\0\0004\0\0\0"..., 936) = 936
mmap2(NULL, 106496, PROT_READ|PROT_EXEC, MAP_PRIVATE, 3, 0) = 0xb6e88000
mmap2(0xb6ea1000, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED, 3, 0x9000) = 0xb6ea1000
close(3) = 0
mprotect(0xb6f15000, 4096, PROT_READ) = 0
mprotect(0x38000, 4096, PROT_READ) = 0
clock_gettime(CLOCK_REALTIME, {1490822261, 558395832}) = 0
open(".", O_RDWR|O_LARGEFILE|O_DIRECTORY|O_TMPFILE) = -1 EOPNOTSUPP (Not supported)
writev(2, [{iov_base="", iov_len=0}, {iov_base=".", iov_len=1}], 2.) = 1
writev(2, [{iov_base="", iov_len=0}, {iov_base=":", iov_len=1}], 2:) = 1
writev(2, [{iov_base="", iov_len=0}, {iov_base=" ", iov_len=1}], 2 ) = 1
writev(2, [{iov_base="", iov_len=0}, {iov_base="Not supported", iov_len=13}], 2Not supported) = 13
writev(2, [{iov_base="", iov_len=0}, {iov_base="\n", iov_len=1}], 2
) = 1
exit_group(1) = ?
+++ exited with 1 +++
Need some sleep before looking into what might be going on here.
Ralph
>
> That's the gist of xfstest generic/004.
>
> You should expect the same behavior of corrupted fs after boot.
>
> This issue should be reproduced since kernel 4.9 when ubifs tmpfile
> support was added.
>
> I hope this info can help the ubifs developers fix the problem
> independently from overlayfs setup.
>
> Thanks,
> Amir.
Ralph,
Am 29.03.2017 um 23:26 schrieb Ralph Sennhauser:
>> # create and link a tmpfile - then remove it
>> sudo rm -rf foo; sudo xfs_io -T -c "flink foo" . ; ls -l foo; sudo rm
>> foo
>
> next-20170328, obviously without your patch:
>
> # xfs_io -T -c "flink foo" .
> .: Not supported
-T tells xfs_io to create a tmpfile but you have it disabled in UBIFS.
Anyway, can you please test the attached patch?
Amir was right, UBIFS misses a corner case in ubifs_link(). ;-\
I think I understand also why we never noticed this in xfstests, UBIFS prints
only a non-fatal error while umounting the filesystem in this case.
But will test tomorrow in more detail, need some sleep now.
diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
index 30825d882aa9..95e348a2da29 100644
--- a/fs/ubifs/dir.c
+++ b/fs/ubifs/dir.c
@@ -748,6 +748,9 @@ static int ubifs_link(struct dentry *old_dentry, struct inode *dir,
goto out_fname;
lock_2_inodes(dir, inode);
+ if (inode->i_nlink == 0)
+ ubifs_delete_orphan(c, inode->i_ino);
+
inc_nlink(inode);
ihold(inode);
inode->i_ctime = ubifs_current_time(inode);
Thanks,
//richard
Hi Richard,
On Thu, 30 Mar 2017 00:15:31 +0200
Richard Weinberger <[email protected]> wrote:
> Ralph,
>
> Am 29.03.2017 um 23:26 schrieb Ralph Sennhauser:
> >> # create and link a tmpfile - then remove it
> >> sudo rm -rf foo; sudo xfs_io -T -c "flink foo" . ; ls -l foo; sudo
> >> rm foo
> >
> > next-20170328, obviously without your patch:
> >
> > # xfs_io -T -c "flink foo" .
> > .: Not supported
>
> -T tells xfs_io to create a tmpfile but you have it disabled in UBIFS.
Bash history says I booted the other partition than I flashed (uname
just happened to match), the downside of the dual boot layout. :)
# rm -rf foo
# xfs_io -T -c "flink foo" .
# ls -l foo
-rw------- 1 root root 0 Mar 30 02:00 foo
# rm foo
[ 305.001436] UBIFS error (ubi0:0 pid 2493): ubifs_add_orphan: orphaned twice
However, unlike with the overlay setup the filesystem can be mounted
just fine without imediatly visible side effects.
>
> Anyway, can you please test the attached patch?
> Amir was right, UBIFS misses a corner case in ubifs_link(). ;-\
>
> I think I understand also why we never noticed this in xfstests,
> UBIFS prints only a non-fatal error while umounting the filesystem in
> this case. But will test tomorrow in more detail, need some sleep now.
>
> diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
> index 30825d882aa9..95e348a2da29 100644
> --- a/fs/ubifs/dir.c
> +++ b/fs/ubifs/dir.c
> @@ -748,6 +748,9 @@ static int ubifs_link(struct dentry *old_dentry,
> struct inode *dir, goto out_fname;
>
> lock_2_inodes(dir, inode);
> + if (inode->i_nlink == 0)
> + ubifs_delete_orphan(c, inode->i_ino);
> +
> inc_nlink(inode);
> ihold(inode);
> inode->i_ctime = ubifs_current_time(inode);
>
> Thanks,
> //richard
With this patch I'm no longer able to reproduce _this_ issue, however,
rename no longer works either:
# mv /etc/config/wireless /etc/config/wireless.back
mv: can't rename '/etc/config/wireless': Invalid argument
# ls /etc/config/wireless
/etc/config/wireless
# rm /etc/config/wireless
# ls /etc/config/wireless
ls: /etc/config/wireless: No such file or directory
Thanks
Ralph
On Thu, Mar 30, 2017 at 8:53 AM, Ralph Sennhauser
<[email protected]> wrote:
> Hi Richard,
>
> On Thu, 30 Mar 2017 00:15:31 +0200
> Richard Weinberger <[email protected]> wrote:
>
>> Ralph,
>>
>> Am 29.03.2017 um 23:26 schrieb Ralph Sennhauser:
>> >> # create and link a tmpfile - then remove it
>> >> sudo rm -rf foo; sudo xfs_io -T -c "flink foo" . ; ls -l foo; sudo
>> >> rm foo
>> >
>> > next-20170328, obviously without your patch:
>> >
>> > # xfs_io -T -c "flink foo" .
>> > .: Not supported
>>
>> -T tells xfs_io to create a tmpfile but you have it disabled in UBIFS.
>
> Bash history says I booted the other partition than I flashed (uname
> just happened to match), the downside of the dual boot layout. :)
>
> # rm -rf foo
> # xfs_io -T -c "flink foo" .
> # ls -l foo
> -rw------- 1 root root 0 Mar 30 02:00 foo
> # rm foo
> [ 305.001436] UBIFS error (ubi0:0 pid 2493): ubifs_add_orphan: orphaned twice
>
> However, unlike with the overlay setup the filesystem can be mounted
> just fine without imediatly visible side effects.
>
Maybe try to rename instead of rm . that's how you reproduces with overlayfs.
>>
>> Anyway, can you please test the attached patch?
>> Amir was right, UBIFS misses a corner case in ubifs_link(). ;-\
>>
>> I think I understand also why we never noticed this in xfstests,
>> UBIFS prints only a non-fatal error while umounting the filesystem in
>> this case. But will test tomorrow in more detail, need some sleep now.
>>
>> diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
>> index 30825d882aa9..95e348a2da29 100644
>> --- a/fs/ubifs/dir.c
>> +++ b/fs/ubifs/dir.c
>> @@ -748,6 +748,9 @@ static int ubifs_link(struct dentry *old_dentry,
>> struct inode *dir, goto out_fname;
>>
>> lock_2_inodes(dir, inode);
>> + if (inode->i_nlink == 0)
>> + ubifs_delete_orphan(c, inode->i_ino);
>> +
>> inc_nlink(inode);
>> ihold(inode);
>> inode->i_ctime = ubifs_current_time(inode);
>>
>> Thanks,
>> //richard
>
> With this patch I'm no longer able to reproduce _this_ issue, however,
> rename no longer works either:
>
> # mv /etc/config/wireless /etc/config/wireless.back
> mv: can't rename '/etc/config/wireless': Invalid argument
> # ls /etc/config/wireless
> /etc/config/wireless
> # rm /etc/config/wireless
> # ls /etc/config/wireless
> ls: /etc/config/wireless: No such file or directory
>
>
> Thanks
> Ralph
Ralph,
Am 30.03.2017 um 07:53 schrieb Ralph Sennhauser:
> With this patch I'm no longer able to reproduce _this_ issue, however,
> rename no longer works either:
>
> # mv /etc/config/wireless /etc/config/wireless.back
> mv: can't rename '/etc/config/wireless': Invalid argument
> # ls /etc/config/wireless
> /etc/config/wireless
> # rm /etc/config/wireless
> # ls /etc/config/wireless
> ls: /etc/config/wireless: No such file or directory
>
Please apply this one too:
http://lists.infradead.org/pipermail/linux-mtd/2017-March/072613.html
Thanks,
//richard
Hi Richard,
On Thu, 30 Mar 2017 08:56:57 +0200
Richard Weinberger <[email protected]> wrote:
>
> Ralph,
> Please apply this
> fix:http://lists.infradead.org/pipermail/linux-mtd/2017-March/072613.html
>
No longer able to observe an issue with this one on top. The two
patches seem all that was needed to get things back to a working state.
Do you want me to test anything else than linux-next?
Thanks
Ralph
>
> Von meinem Samsung Gerät gesendet.
>
> -------- Ursprüngliche Nachricht --------
> Von: Ralph Sennhauser <[email protected]>
> Datum: 30.03.17 07:53 (GMT+01:00)
> An: Richard Weinberger <[email protected]>
> Cc: Amir Goldstein <[email protected]>, Miklos Szeredi
> <[email protected]>, [email protected], linux-kernel
> <[email protected]>, [email protected],
> [email protected], Artem Bityutskiy <[email protected]>,
> Adrian Hunter <[email protected]>, David Oberhollenzer
> <[email protected]> Betreff: Re: [REGRESSION 4.11] Commit
> d8514d8edb5b ("ovl: copy up regular
file using O_TMPFILE") breaks ubifs
>
> Hi Richard,
>
> On Thu, 30 Mar 2017 00:15:31 +0200
> Richard Weinberger <[email protected]> wrote:
>
> > Ralph,
> >
> > Am 29.03.2017 um 23:26 schrieb Ralph Sennhauser:
> > >> # create and link a tmpfile - then remove it
> > >> sudo rm -rf foo; sudo xfs_io -T -c "flink foo" . ; ls -l foo;
> > >> sudo rm foo
> > >
> > > next-20170328, obviously without your patch:
> > >
> > > # xfs_io -T -c "flink foo" .
> > > .: Not supported
> >
> > -T tells xfs_io to create a tmpfile but you have it disabled in
> > UBIFS.
>
> Bash history says I booted the other partition than I flashed (uname
> just happened to match), the downside of the dual boot layout. :)
>
> # rm -rf foo
> # xfs_io -T -c "flink foo" .
> # ls -l foo
> -rw------- 1 root root 0 Mar 30 02:00 foo
> # rm foo
> [ 305.001436] UBIFS error (ubi0:0 pid 2493): ubifs_add_orphan:
> orphaned twice
>
> However, unlike with the overlay setup the filesystem can be mounted
> just fine without imediatly visible side effects.
>
> >
> > Anyway, can you please test the attached patch?
> > Amir was right, UBIFS misses a corner case in ubifs_link(). ;-\
> >
> > I think I understand also why we never noticed this in xfstests,
> > UBIFS prints only a non-fatal error while umounting the filesystem
> > in this case. But will test tomorrow in more detail, need some
> > sleep now.
> >
> > diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
> > index 30825d882aa9..95e348a2da29 100644
> > --- a/fs/ubifs/dir.c
> > +++ b/fs/ubifs/dir.c
> > @@ -748,6 +748,9 @@ static int ubifs_link(struct dentry *old_dentry,
> > struct inode *dir, goto out_fname;
> >
> > lock_2_inodes(dir, inode);
> > + if (inode->i_nlink == 0)
> > + ubifs_delete_orphan(c, inode->i_ino);
> > +
> > inc_nlink(inode);
> > ihold(inode);
> > inode->i_ctime = ubifs_current_time(inode);
> >
> > Thanks,
> > //richard
>
> With this patch I'm no longer able to reproduce _this_ issue, however,
> rename no longer works either:
>
> # mv /etc/config/wireless /etc/config/wireless.back
> mv: can't rename '/etc/config/wireless': Invalid argument
> # ls /etc/config/wireless
> /etc/config/wireless
> # rm /etc/config/wireless
> # ls /etc/config/wireless
> ls: /etc/config/wireless: No such file or directory
>
>
> Thanks
> Ralph