From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qk0-f196.google.com ([209.85.220.196]:34636 "EHLO mail-qk0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751374AbcFXN4q (ORCPT ); Fri, 24 Jun 2016 09:56:46 -0400 Received: by mail-qk0-f196.google.com with SMTP id j2so21047446qkf.1 for ; Fri, 24 Jun 2016 06:56:46 -0700 (PDT) Date: Fri, 24 Jun 2016 09:56:13 -0400 From: Zheng Lv To: OGAWA Hirofumi Cc: linux-fsdevel@vger.kernel.org Subject: Re: [PATCH] fat: check whether fs size exceeds device size Message-ID: <20160624135613.GA22429@ubuntu-sun> References: <1466758690-12354-1-git-send-email-lv.zheng.2015@gmail.com> <8737o3lyla.fsf@mail.parknet.co.jp> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <8737o3lyla.fsf@mail.parknet.co.jp> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: 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