Re: [PATCH 4.14 024/110] btrfs: use proper endianness accessors for super_copy
On Thu, Mar 15, 2018 at 07:55:42PM +0100, Christoph Biedl wrote: > Greg Kroah-Hartman wrote... > > > 4.14-stable review patch. If anyone has any objections, please let me know. > > > commit 3c181c12c431fe33b669410d663beb9cceefcd1b upstream. > (...) > > > If the filesystem is always used on a same endian host, this will not > > be a problem. > > >From my observations I cannot quite subscribe to that. > > On big-endian systems, this change intruduces severe corruption, > resulting in complete loss of the data on the used block device. > > Steps to reproduce (tested on ppc/powerpc and parisc/hppa): > > # mkfs.btrfs $DEV > # mount $DEV /mnt/tmp/ > # umount /mnt/tmp/ > > This simple umount corrupts the file system: > > # mount $DEV /mnt/tmp/ > mount: /mnt/tmp: wrong fs type, bad option, bad superblock on $DEV, missing > codepage or helper program, or other error. > > # dmesg: > BTRFS critical (device ): unable to find logical 4294967296 length 4096 > BTRFS critical (device ): unable to find logical 4294967296 length 4096 > BTRFS critical (device ): unable to find logical 18102363734671360 > length 16384 > BTRFS error (device ): failed to read chunk root > BTRFS error (device ): open_ctree failed > > Also fsck is of no help: > > # btrfsck $DEV > Couldn't map the block 18102363734671360 > No mapping for 18102363734671360-18102363734687744 > Couldn't map the block 18102363734671360 > bytenr mismatch, want=18102363734671360, have=0 > ERROR: cannot read chunk root > ERROR: cannot open file system > > > Trying mount or fsck on a little-endian system does not help either. So > I consider the data on that device lost - luckily I use btrfs only for > files where a backup exists all the time. > > > Reverting that change restored the previous error-free behaviour. I > didn't check HEAD, i.e. v4.16-rc5, since the upstream commt was the last > that affected these files. Still I could give this a try if anybody > wishes so. That sucks. Can you test Linus's tree to verify the problem is there? I'll gladly revert this if Linus's tree also gets the revert, I don't want you to hit this when you upgrade to a newer kernel. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4.14 024/110] btrfs: use proper endianness accessors for super_copy
On Fri, Mar 16, 2018 at 01:30:49PM +0100, Greg Kroah-Hartman wrote: > On Thu, Mar 15, 2018 at 07:55:42PM +0100, Christoph Biedl wrote: > > Greg Kroah-Hartman wrote... > > > > > 4.14-stable review patch. If anyone has any objections, please let me > > > know. > > > > > commit 3c181c12c431fe33b669410d663beb9cceefcd1b upstream. > > (...) > > > > > If the filesystem is always used on a same endian host, this will not > > > be a problem. > > > > >From my observations I cannot quite subscribe to that. > > > > On big-endian systems, this change intruduces severe corruption, > > resulting in complete loss of the data on the used block device. > > > > Steps to reproduce (tested on ppc/powerpc and parisc/hppa): > > > > # mkfs.btrfs $DEV > > # mount $DEV /mnt/tmp/ > > # umount /mnt/tmp/ > > > > This simple umount corrupts the file system: > > > > # mount $DEV /mnt/tmp/ > > mount: /mnt/tmp: wrong fs type, bad option, bad superblock on $DEV, missing > > codepage or helper program, or other error. > > > > # dmesg: > > BTRFS critical (device ): unable to find logical 4294967296 length 4096 > > BTRFS critical (device ): unable to find logical 4294967296 length 4096 > > BTRFS critical (device ): unable to find logical 18102363734671360 > > length 16384 > > BTRFS error (device ): failed to read chunk root > > BTRFS error (device ): open_ctree failed > > > > Also fsck is of no help: > > > > # btrfsck $DEV > > Couldn't map the block 18102363734671360 > > No mapping for 18102363734671360-18102363734687744 > > Couldn't map the block 18102363734671360 > > bytenr mismatch, want=18102363734671360, have=0 > > ERROR: cannot read chunk root > > ERROR: cannot open file system > > > > > > Trying mount or fsck on a little-endian system does not help either. So > > I consider the data on that device lost - luckily I use btrfs only for > > files where a backup exists all the time. > > > > > > Reverting that change restored the previous error-free behaviour. I > > didn't check HEAD, i.e. v4.16-rc5, since the upstream commt was the last > > that affected these files. Still I could give this a try if anybody > > wishes so. > > That sucks. Can you test Linus's tree to verify the problem is there? > I'll gladly revert this if Linus's tree also gets the revert, I don't > want you to hit this when you upgrade to a newer kernel. I'll push a fix for the upcoming rc but I think it would be better to remove the broken patch from stable kernels ASAP, so I'd recommend to revert it now. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4.14 024/110] btrfs: use proper endianness accessors for super_copy
On Fri, Mar 16, 2018 at 02:22:02PM +0100, David Sterba wrote: > On Fri, Mar 16, 2018 at 01:30:49PM +0100, Greg Kroah-Hartman wrote: > > On Thu, Mar 15, 2018 at 07:55:42PM +0100, Christoph Biedl wrote: > > > Greg Kroah-Hartman wrote... > > > > > > > 4.14-stable review patch. If anyone has any objections, please let me > > > > know. > > > > > > > commit 3c181c12c431fe33b669410d663beb9cceefcd1b upstream. > > > (...) > > > > > > > If the filesystem is always used on a same endian host, this will not > > > > be a problem. > > > > > > >From my observations I cannot quite subscribe to that. > > > > > > On big-endian systems, this change intruduces severe corruption, > > > resulting in complete loss of the data on the used block device. > > > > > > Steps to reproduce (tested on ppc/powerpc and parisc/hppa): > > > > > > # mkfs.btrfs $DEV > > > # mount $DEV /mnt/tmp/ > > > # umount /mnt/tmp/ > > > > > > This simple umount corrupts the file system: > > > > > > # mount $DEV /mnt/tmp/ > > > mount: /mnt/tmp: wrong fs type, bad option, bad superblock on $DEV, > > > missing codepage or helper program, or other error. > > > > > > # dmesg: > > > BTRFS critical (device ): unable to find logical 4294967296 length > > > 4096 > > > BTRFS critical (device ): unable to find logical 4294967296 length > > > 4096 > > > BTRFS critical (device ): unable to find logical 18102363734671360 > > > length 16384 > > > BTRFS error (device ): failed to read chunk root > > > BTRFS error (device ): open_ctree failed > > > > > > Also fsck is of no help: > > > > > > # btrfsck $DEV > > > Couldn't map the block 18102363734671360 > > > No mapping for 18102363734671360-18102363734687744 > > > Couldn't map the block 18102363734671360 > > > bytenr mismatch, want=18102363734671360, have=0 > > > ERROR: cannot read chunk root > > > ERROR: cannot open file system > > > > > > > > > Trying mount or fsck on a little-endian system does not help either. So > > > I consider the data on that device lost - luckily I use btrfs only for > > > files where a backup exists all the time. > > > > > > > > > Reverting that change restored the previous error-free behaviour. I > > > didn't check HEAD, i.e. v4.16-rc5, since the upstream commt was the last > > > that affected these files. Still I could give this a try if anybody > > > wishes so. > > > > That sucks. Can you test Linus's tree to verify the problem is there? > > I'll gladly revert this if Linus's tree also gets the revert, I don't > > want you to hit this when you upgrade to a newer kernel. > > I'll push a fix for the upcoming rc but I think it would be better to > remove the broken patch from stable kernels ASAP, so I'd recommend to > revert it now. Now reverted, thanks. greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4.14 024/110] btrfs: use proper endianness accessors for super_copy
On 03/16/2018 10:02 PM, Greg Kroah-Hartman wrote: On Fri, Mar 16, 2018 at 02:22:02PM +0100, David Sterba wrote: On Fri, Mar 16, 2018 at 01:30:49PM +0100, Greg Kroah-Hartman wrote: On Thu, Mar 15, 2018 at 07:55:42PM +0100, Christoph Biedl wrote: Greg Kroah-Hartman wrote... 4.14-stable review patch. If anyone has any objections, please let me know. commit 3c181c12c431fe33b669410d663beb9cceefcd1b upstream. (...) If the filesystem is always used on a same endian host, this will not be a problem. >From my observations I cannot quite subscribe to that. On big-endian systems, this change intruduces severe corruption, resulting in complete loss of the data on the used block device. Steps to reproduce (tested on ppc/powerpc and parisc/hppa): # mkfs.btrfs $DEV # mount $DEV /mnt/tmp/ # umount /mnt/tmp/ This simple umount corrupts the file system: # mount $DEV /mnt/tmp/ mount: /mnt/tmp: wrong fs type, bad option, bad superblock on $DEV, missing codepage or helper program, or other error. # dmesg: BTRFS critical (device ): unable to find logical 4294967296 length 4096 BTRFS critical (device ): unable to find logical 4294967296 length 4096 BTRFS critical (device ): unable to find logical 18102363734671360 length 16384 BTRFS error (device ): failed to read chunk root BTRFS error (device ): open_ctree failed Also fsck is of no help: # btrfsck $DEV Couldn't map the block 18102363734671360 No mapping for 18102363734671360-18102363734687744 Couldn't map the block 18102363734671360 bytenr mismatch, want=18102363734671360, have=0 ERROR: cannot read chunk root ERROR: cannot open file system Trying mount or fsck on a little-endian system does not help either. So I consider the data on that device lost - luckily I use btrfs only for files where a backup exists all the time. Reverting that change restored the previous error-free behaviour. I didn't check HEAD, i.e. v4.16-rc5, since the upstream commt was the last that affected these files. Still I could give this a try if anybody wishes so. That sucks. Can you test Linus's tree to verify the problem is there? I'll gladly revert this if Linus's tree also gets the revert, I don't want you to hit this when you upgrade to a newer kernel. I'll push a fix for the upcoming rc but I think it would be better to remove the broken patch from stable kernels ASAP, so I'd recommend to revert it now. Now reverted, thanks. Thanks ! Sorry for the mess. -Anand greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4.14 024/110] btrfs: use proper endianness accessors for super_copy
Greg Kroah-Hartman wrote... > On Thu, Mar 15, 2018 at 07:55:42PM +0100, Christoph Biedl wrote: > > > commit 3c181c12c431fe33b669410d663beb9cceefcd1b upstream. > > On big-endian systems, this change intruduces severe corruption, > > resulting in complete loss of the data on the used block device. > That sucks. Can you test Linus's tree to verify the problem is there? > I'll gladly revert this if Linus's tree also gets the revert, I don't > want you to hit this when you upgrade to a newer kernel. Confirmed: The problem is, err ... was in Linus' tree as well. The rather recent commit 8f5fd927c3a7 reverted the change, after that everything is as expected again. Looking at the original commit, I don't have a clue why things go wrong so horribly - otherwise don't be afraid of my data. I took this as a chance to verify my data recovery procedure, with success. Christoph -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4.14 024/110] btrfs: use proper endianness accessors for super_copy
On Sat, Mar 17, 2018 at 06:27:15PM +0100, Christoph Biedl wrote: > Greg Kroah-Hartman wrote... > > > On Thu, Mar 15, 2018 at 07:55:42PM +0100, Christoph Biedl wrote: > > > > > commit 3c181c12c431fe33b669410d663beb9cceefcd1b upstream. > > > > On big-endian systems, this change intruduces severe corruption, > > > resulting in complete loss of the data on the used block device. > > > That sucks. Can you test Linus's tree to verify the problem is there? > > I'll gladly revert this if Linus's tree also gets the revert, I don't > > want you to hit this when you upgrade to a newer kernel. > > Confirmed: The problem is, err ... was in Linus' tree as well. The > rather recent commit 8f5fd927c3a7 reverted the change, after that > everything is as expected again. Thanks for checking. > Looking at the original commit, I don't have a clue why things go wrong > so horribly It's a half endianness conversion. The plain in-memory structures are in LE and has to be accessed via the helpers, super block copy and the root item. The commit adds only one half of the conversion, that naturally does not exhibit on LE, because the macros are no-op. Originally, the items were stored from the on-disk type to on-disk type, regardless of the CPU: super->chunk_root = root_item->bytenr; The patch should have added conversion of both values, like btrfs_set_super_chunk_root(super, btrfs_root_bytenr(root_item)); and not btrfs_set_super_chunk_root(super, root_item->bytenr); It's not very obvious from the context though, typically there's one structure that needs the accessors and is set from a value that's in CPU byteorder. I think that the exception to this pattern confused all involved developers. The root_item members are annotated as __le64 that should be caught by sparse/smatch checker in the buggy case, but we don't run the checkers every the time. > - otherwise don't be afraid of my data. I took this as a > chance to verify my data recovery procedure, with success. Should that not be the case, the damaged items in superblock can be byteswapped back. That's 6 x u64 at most, I have a tool for that now. Thanks again for the report and sorry for the trouble. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4.14 024/110] btrfs: use proper endianness accessors for super_copy
On 03/20/2018 03:32 AM, David Sterba wrote: On Sat, Mar 17, 2018 at 06:27:15PM +0100, Christoph Biedl wrote: Greg Kroah-Hartman wrote... On Thu, Mar 15, 2018 at 07:55:42PM +0100, Christoph Biedl wrote: commit 3c181c12c431fe33b669410d663beb9cceefcd1b upstream. On big-endian systems, this change intruduces severe corruption, resulting in complete loss of the data on the used block device. That sucks. Can you test Linus's tree to verify the problem is there? I'll gladly revert this if Linus's tree also gets the revert, I don't want you to hit this when you upgrade to a newer kernel. Confirmed: The problem is, err ... was in Linus' tree as well. The rather recent commit 8f5fd927c3a7 reverted the change, after that everything is as expected again. Thanks for checking. Looking at the original commit, I don't have a clue why things go wrong so horribly It's a half endianness conversion. The plain in-memory structures are in LE and has to be accessed via the helpers, super block copy and the root item. The commit adds only one half of the conversion, that naturally does not exhibit on LE, because the macros are no-op. Originally, the items were stored from the on-disk type to on-disk type, regardless of the CPU: super->chunk_root = root_item->bytenr; The patch should have added conversion of both values, like btrfs_set_super_chunk_root(super, btrfs_root_bytenr(root_item)); and not btrfs_set_super_chunk_root(super, root_item->bytenr); It's not very obvious from the context though, typically there's one structure that needs the accessors and is set from a value that's in CPU byteorder. I think that the exception to this pattern confused all involved developers. The root_item members are annotated as __le64 that should be caught by sparse/smatch checker in the buggy case, but we don't run the checkers every the time. Ah! the RC is at the other side of the equation. Makes sense to me. Thanks. - otherwise don't be afraid of my data. I took this as a chance to verify my data recovery procedure, with success. Should that not be the case, the damaged items in superblock can be byteswapped back. That's 6 x u64 at most, I have a tool for that now. Thanks again for the report and sorry for the trouble. It's entirely my mistake. My apologies for the inconvenience. Thanks, Anand -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
