Re: [PATCH 1/2] Btrfs: add more valid checks for superblock

[Liu Bo <bo.li.liu@oracle.com>]

On Mon, May 09, 2016 at 09:31:37AM +0800, Qu Wenruo wrote:
> 
> 
> David Sterba wrote on 2016/05/06 16:35 +0200:
> > On Thu, May 05, 2016 at 09:08:54AM +0800, Qu Wenruo wrote:
> > > > > An early check can compare against some reasonable value, but the
> > > > > total_bytes value must be equal to the sum of all device sizes
> > > > > (disk_total_bytes). I'm not sure if we have enough information to verify
> > > > > that at this point though.
> > > > 
> > > > That's what I had in mind, the problem is that only the first device information is recorded in superblock.
> > > > 
> > > > At this moment We have device_num but we don't know the size of other devices.
> > > > 
> > > > Thanks,
> > > > 
> > > > -liubo
> > > > 
> > > > 
> > > What about error out if we found sb->total_bytes <
> > > sb->dev_item->total_bytes?
> > > 
> > > As we are just doing early check, no need to be comprehensive, but spot
> > > obvious problem.
> > 
> > Ok.

I'm gonna check for total_bytes and num_devices after loading chunk
tree.

> > 
> > > For exact device_num and sb->total_bytes, we may do post check when
> > > device tree are loaded?
> > > Splitting early_check() and post_check() seems valid for me.
> > > (Also I prefer post_check() just warning, not forced exit)
> > 
> > Why just warning? Superblock total_bytes and device sizes must be
> > correct, otherwise all sorts of operations can fail randomly.
> > 
> > 
> Because if we exit, we can't even do fsck.
> 
> Maybe we need a new flag to control whether exit or warn at post_check().
> 

> Thanks,
> Qu
>

IMHO for kernel part, we have to exit in order to avoid any panic due to those invalid value.

For fsck code, we can go forth and back to fix them.  In fact I don't
think fsck could work out anything, as superblock checksum has _matched_
but the values inside superblock are invalid, in this case we cannot trust
 other parts in this FS image, then how can we expect fsck to fix it by reading
other parts?

Thanks,

-liubo