Re: [PATCH] fat: check whether fs size exceeds device size

[Zheng Lv <lv.zheng.2015@gmail.com>]

On Fri, Jun 24, 2016 at 06:34:25PM +0900, OGAWA Hirofumi wrote:
> > +	device_sectors = sb->s_bdev->bd_inode->i_size / logical_sector_size;
> > +	if (device_sectors && total_sectors > device_sectors) {
> > +		fat_msg(sb, KERN_ERR, "total sectors %u "
> > +			"exceeds size of device (%llu sectors)",
> > +			total_sectors, device_sectors);
> > +		goto out_invalid;
> 
> Hm, it is a bit hard to think what to do. But I guess it is better to
> allow access to rescue some files. (Yes, it may lost new data. But
> read-only or in-place update is safe.)

I would like to list the reasons it's better not to allow mounting.

 - The "attempt to access beyond end of device" error would fill the kernel
   log. It's drivers' business to prevent such kind of errors.
 - Clever data rescuers won't mount the damaged device. They would instead
   mount a copy of the broken image. It isn't much work to "truncate" the file
   to larger after they receive the message. 
 - It's at least unsafe to allow mounting rw a truncated device.
 - ext4 driver forbids mounting a truncated device, too.

In fact, the code was basically copied from ext4 fs driver. :-)

Reference: v4.7-rc4 fs/ext4/super.c:3605
>	blocks_count = sb->s_bdev->bd_inode->i_size >> sb->s_blocksize_bits;
>	if (blocks_count && ext4_blocks_count(es) > blocks_count) {
>		ext4_msg(sb, KERN_WARNING, "bad geometry: block count %llu "
>		       "exceeds size of device (%llu blocks)",
>		       ext4_blocks_count(es), blocks_count);
>		goto failed_mount;
>	}

On Fri, Jun 24, 2016 at 06:34:25PM +0900, OGAWA Hirofumi wrote:
> > +	} else if (device_sectors && total_sectors < device_sectors) {
> > +		fat_msg(sb, KERN_INFO, "%llu unused sectors at end of device",
> > +			device_sectors - total_sectors);
> 
> This is legal setup. So, probably, should not pollute log for each mount.

I totally agree with you.

Zheng Lv