* [PATCH] btrfs: Don't continue mounting when superblock csum mismatches even generation is less than 10. @ 2014-06-24 8:49 Qu Wenruo 2014-08-07 2:51 ` Qu Wenruo 0 siblings, 1 reply; 7+ messages in thread From: Qu Wenruo @ 2014-06-24 8:49 UTC (permalink / raw) To: linux-btrfs Revert kernel commit 667e7d94a1683661cff5fe9a0fa0d7f8fdd2c007. (Btrfs: allow superblock mismatch from older mkfs by Chris Mason) Above commit will cause disaster if someone try to mount a newly created but later corrupted btrfs filesystem. And before btrfs entered mainline, btrfs-progs has already superblock checksum. See btrfs-progs commit: 5ccd1715fa2eaad0b26037bb53706779c8c93b5f (superblock duplication by Yan Zheng). Before commit 5ccd17, mkfs.btrfs uses 16K as super offset, while current btrfs uses 64K super offset, anyway old btrfs without super csum will not be mountable due to the change of super offset. So backward compatibility is not a problem. Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com> --- fs/btrfs/disk-io.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c index 8bb4aa1..dbfb2a3 100644 --- a/fs/btrfs/disk-io.c +++ b/fs/btrfs/disk-io.c @@ -400,12 +400,6 @@ static int btrfs_check_super_csum(char *raw_disk_sb) if (memcmp(raw_disk_sb, result, csum_size)) ret = 1; - - if (ret && btrfs_super_generation(disk_sb) < 10) { - printk(KERN_WARNING - "BTRFS: super block crcs don't match, older mkfs detected\n"); - ret = 0; - } } if (csum_type >= ARRAY_SIZE(btrfs_csum_sizes)) { -- 2.0.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] btrfs: Don't continue mounting when superblock csum mismatches even generation is less than 10. 2014-06-24 8:49 [PATCH] btrfs: Don't continue mounting when superblock csum mismatches even generation is less than 10 Qu Wenruo @ 2014-08-07 2:51 ` Qu Wenruo 2014-08-19 17:18 ` David Sterba 2014-08-19 19:48 ` Chris Mason 0 siblings, 2 replies; 7+ messages in thread From: Qu Wenruo @ 2014-08-07 2:51 UTC (permalink / raw) To: linux-btrfs; +Cc: David Sterba, Josef Bacik, Chris Mason It seems that the patch is rejected in patchwork, Could any one tell me the reason? Thanks, Qu -------- Original Message -------- Subject: [PATCH] btrfs: Don't continue mounting when superblock csum mismatches even generation is less than 10. From: Qu Wenruo <quwenruo@cn.fujitsu.com> To: linux-btrfs@vger.kernel.org Date: 2014年06月24日 16:49 > Revert kernel commit 667e7d94a1683661cff5fe9a0fa0d7f8fdd2c007. > (Btrfs: allow superblock mismatch from older mkfs by Chris Mason) > > Above commit will cause disaster if someone try to mount a newly created but > later corrupted btrfs filesystem. > > And before btrfs entered mainline, btrfs-progs has already superblock > checksum. See btrfs-progs commit: 5ccd1715fa2eaad0b26037bb53706779c8c93b5f > (superblock duplication by Yan Zheng). > Before commit 5ccd17, mkfs.btrfs uses 16K as super offset, while current btrfs > uses 64K super offset, anyway old btrfs without super csum will not be > mountable due to the change of super offset. > > So backward compatibility is not a problem. > > Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com> > --- > fs/btrfs/disk-io.c | 6 ------ > 1 file changed, 6 deletions(-) > > diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c > index 8bb4aa1..dbfb2a3 100644 > --- a/fs/btrfs/disk-io.c > +++ b/fs/btrfs/disk-io.c > @@ -400,12 +400,6 @@ static int btrfs_check_super_csum(char *raw_disk_sb) > > if (memcmp(raw_disk_sb, result, csum_size)) > ret = 1; > - > - if (ret && btrfs_super_generation(disk_sb) < 10) { > - printk(KERN_WARNING > - "BTRFS: super block crcs don't match, older mkfs detected\n"); > - ret = 0; > - } > } > > if (csum_type >= ARRAY_SIZE(btrfs_csum_sizes)) { ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] btrfs: Don't continue mounting when superblock csum mismatches even generation is less than 10. 2014-08-07 2:51 ` Qu Wenruo @ 2014-08-19 17:18 ` David Sterba 2014-08-20 1:16 ` Qu Wenruo 2014-08-19 19:48 ` Chris Mason 1 sibling, 1 reply; 7+ messages in thread From: David Sterba @ 2014-08-19 17:18 UTC (permalink / raw) To: Qu Wenruo; +Cc: linux-btrfs, David Sterba, Josef Bacik, Chris Mason On Thu, Aug 07, 2014 at 10:51:15AM +0800, Qu Wenruo wrote: > It seems that the patch is rejected in patchwork, It was not me :) > Could any one tell me the reason? I'd understand that the patch is no longer needed after the original problem went away, but it's not what you describe in your changelog. >From that point the reason might not be compelling. > >Above commit will cause disaster if someone try to mount a newly created but > >later corrupted btrfs filesystem. The generation after mkfs is something like 4 or 5, this means that the corruption would have to happen in the first few transaction commits, this is unlikely and the filesystem will be probably fairly empty at that time. If the concern is about corrupted generation counter itself in the superblock, then yes this could hurt. It's still possible to compare the 1st superblock with the copies, the one at offset 64M is available in 99%, there are enough data to make a decision what's actually corrupted. This could catch more corruption than just the generation counter. >From the output of btrfs-show-super: generation 56392 chunk_root_generation 56392 cache_generation 56392 uuid_tree_generation 56392 the generation is duplicated several times, so a minimal patch could be to do additional comparison with the others. > >And before btrfs entered mainline, btrfs-progs has already superblock > >checksum. See btrfs-progs commit: 5ccd1715fa2eaad0b26037bb53706779c8c93b5f > >(superblock duplication by Yan Zheng). The superblock checksum was not calculated the same way as in kernel, but with the missing check this was not detected. > >Before commit 5ccd17, mkfs.btrfs uses 16K as super offset, while current btrfs > >uses 64K super offset, anyway old btrfs without super csum will not be > >mountable due to the change of super offset. > > > >So backward compatibility is not a problem. Superblocks at offset 16k are not supported anymore AFAICT. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] btrfs: Don't continue mounting when superblock csum mismatches even generation is less than 10. 2014-08-19 17:18 ` David Sterba @ 2014-08-20 1:16 ` Qu Wenruo 0 siblings, 0 replies; 7+ messages in thread From: Qu Wenruo @ 2014-08-20 1:16 UTC (permalink / raw) To: dsterba, linux-btrfs, Josef Bacik, Chris Mason -------- Original Message -------- Subject: Re: [PATCH] btrfs: Don't continue mounting when superblock csum mismatches even generation is less than 10. From: David Sterba <dsterba@suse.cz> To: Qu Wenruo <quwenruo@cn.fujitsu.com> Date: 2014年08月20日 01:18 > On Thu, Aug 07, 2014 at 10:51:15AM +0800, Qu Wenruo wrote: >> It seems that the patch is rejected in patchwork, > It was not me :) > >> Could any one tell me the reason? > I'd understand that the patch is no longer needed after the original > problem went away, but it's not what you describe in your changelog. > From that point the reason might not be compelling. > >>> Above commit will cause disaster if someone try to mount a newly created but >>> later corrupted btrfs filesystem. > The generation after mkfs is something like 4 or 5, this means that the > corruption would have to happen in the first few transaction commits, > this is unlikely and the filesystem will be probably fairly empty at > that time. > > If the concern is about corrupted generation counter itself in the > superblock, then yes this could hurt. > > It's still possible to compare the 1st superblock with the copies, the > one at offset 64M is available in 99%, there are enough data to make a > decision what's actually corrupted. This could catch more corruption > than just the generation counter. > > From the output of btrfs-show-super: > > generation 56392 > chunk_root_generation 56392 > cache_generation 56392 > uuid_tree_generation 56392 > > the generation is duplicated several times, so a minimal patch could be > to do additional comparison with the others. Thanks for the explaination. But in fact, when investigating some bugs (not kernel bugzilla but proprietary one), I found not only one but two disk images whose superblock csum doesn't match and a lot of values go crazy. For example, num_devices goes to 871878361089 and serval bits diffs in dev_item.fsid and fsid. BTW, cache generation is also crazy. Normally, such superblock should not be mountable since the csum doesn't match. But due to the mentioned commit, the generation (4) is below 10 and kernel just ignore the csum error, and finally, a kernel BUG is triggered, since a lot of things go wrong anything is possible. So I sent the patch and hope to avoid such problem. Thanks, Qu > >>> And before btrfs entered mainline, btrfs-progs has already superblock >>> checksum. See btrfs-progs commit: 5ccd1715fa2eaad0b26037bb53706779c8c93b5f >>> (superblock duplication by Yan Zheng). > The superblock checksum was not calculated the same way as in kernel, > but with the missing check this was not detected. > >>> Before commit 5ccd17, mkfs.btrfs uses 16K as super offset, while current btrfs >>> uses 64K super offset, anyway old btrfs without super csum will not be >>> mountable due to the change of super offset. >>> >>> So backward compatibility is not a problem. > Superblocks at offset 16k are not supported anymore AFAICT. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] btrfs: Don't continue mounting when superblock csum mismatches even generation is less than 10. 2014-08-07 2:51 ` Qu Wenruo 2014-08-19 17:18 ` David Sterba @ 2014-08-19 19:48 ` Chris Mason 2014-08-20 2:34 ` Qu Wenruo 1 sibling, 1 reply; 7+ messages in thread From: Chris Mason @ 2014-08-19 19:48 UTC (permalink / raw) To: Qu Wenruo, linux-btrfs; +Cc: David Sterba, Josef Bacik On 08/06/2014 10:51 PM, Qu Wenruo wrote: > It seems that the patch is rejected in patchwork, > > Could any one tell me the reason? I had nack'd it because I was worried at the time about the super crc errors that Dave had found in the past. Sorry, I really thought I had sent email about it. But Dave has a great point in his reply about validating the super generation. -chris ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] btrfs: Don't continue mounting when superblock csum mismatches even generation is less than 10. 2014-08-19 19:48 ` Chris Mason @ 2014-08-20 2:34 ` Qu Wenruo 2014-08-25 15:28 ` David Sterba 0 siblings, 1 reply; 7+ messages in thread From: Qu Wenruo @ 2014-08-20 2:34 UTC (permalink / raw) To: Chris Mason, linux-btrfs; +Cc: David Sterba, Josef Bacik -------- Original Message -------- Subject: Re: [PATCH] btrfs: Don't continue mounting when superblock csum mismatches even generation is less than 10. From: Chris Mason <clm@fb.com> To: Qu Wenruo <quwenruo@cn.fujitsu.com>, <linux-btrfs@vger.kernel.org> Date: 2014年08月20日 03:48 > On 08/06/2014 10:51 PM, Qu Wenruo wrote: >> It seems that the patch is rejected in patchwork, >> >> Could any one tell me the reason? > I had nack'd it because I was worried at the time about the super crc > errors that Dave had found in the past. Sorry, I really thought I had > sent email about it. > > But Dave has a great point in his reply about validating the super > generation. Thanks for the reason. I'll search and look at Dave's mail and dig into it. Although as mentioned in the reply to David, the main problem is that I found two disk images with crazy values in superblock and wrong csum, but generation is still 4, and ignoring the csum error caused kernel BUG. Thanks, Qu > > -chris ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] btrfs: Don't continue mounting when superblock csum mismatches even generation is less than 10. 2014-08-20 2:34 ` Qu Wenruo @ 2014-08-25 15:28 ` David Sterba 0 siblings, 0 replies; 7+ messages in thread From: David Sterba @ 2014-08-25 15:28 UTC (permalink / raw) To: Qu Wenruo; +Cc: Chris Mason, linux-btrfs, David Sterba, Josef Bacik On Wed, Aug 20, 2014 at 10:34:53AM +0800, Qu Wenruo wrote: > Although as mentioned in the reply to David, > the main problem is that I found two disk images with crazy values in > superblock and wrong csum, > but generation is still 4, and ignoring the csum error caused kernel BUG. Can you please share the dump of the broken superblock (btrfs-show-super)? Thanks. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-08-25 15:28 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-06-24 8:49 [PATCH] btrfs: Don't continue mounting when superblock csum mismatches even generation is less than 10 Qu Wenruo 2014-08-07 2:51 ` Qu Wenruo 2014-08-19 17:18 ` David Sterba 2014-08-20 1:16 ` Qu Wenruo 2014-08-19 19:48 ` Chris Mason 2014-08-20 2:34 ` Qu Wenruo 2014-08-25 15:28 ` David Sterba
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.