On 10/04/2017 07:13 PM, Liu Bo wrote: > On Wed, Oct 04, 2017 at 04:22:28PM +0200, David Sterba wrote: >> On Tue, Oct 03, 2017 at 07:31:10PM +0200, Goffredo Baroncelli wrote: >>> From: Goffredo Baroncelli <kreijack@xxxxxxxxx> >>> >>> Jean-Denis Girard noticed commit c821e7f3 "pass bytes to >>> btrfs_bio_alloc" (https://patchwork.kernel.org/patch/9763081/) introduces a >>> regression on 32 bit machines. >>> When CONFIG_LBDAF is _not_ defined (CONFIG_LBDAF == Support for large >>> (2TB+) block devices and files) sector_t is 32 bit on 32bit machines. >>> >>> In the function submit_extent_page, 'sector' (which is sector_t type) is >>> multiplied by 512 to convert it from sectors to bytes, leading to an >>> overflow when the disk is bigger than 4GB (!). >> >> That's not good. There are some known typedefs that hide the 32bit/64bit >> differences but the LBDAF and sector_t is new to me. Thanks for the >> report and fix, I'll get it to linus/master tree in the next batch so it >> can go to stable tree. >> >> I've seen sector_t used in places where it is not necessary so I'll try >> to minimize the usage and more surprises from the << 9 shifts. >> >> Reviewed-by: David Sterba <dsterba@xxxxxxxx> >> Fixes: c821e7f3 ("btrfs: pass bytes to btrfs_bio_alloc") >> CC: stable@xxxxxxxxxxxxxxx # 4.13+ > > However, this sector_t is passed from its callers, e.g. > > __do_readpage() > { > sector_t sector; > ... > sector = em->block_start >> 9; > ... > } > > if sector_t is 32bit, the above %sector could also lose high bits. > Some cleanups are needed to use u64 directly. If sector_t is 32bit, the kernel can't access disk bigger than 2TB. So I suppose that block_start is less than 4GB. > > Even with this patch, I suspect that there might be errors from block > layer as sector_t is used everywhere in block layer. > > For a btrfs FS that is created and used on 64bit system, it'll be > causing trouble anyway if letting it mount 32bit system, lets refuse > the mount firstly. I think that the check should be: if sizeof(sector_t) < 8 && filesystem_end > 2TB then do_not_mount_it I am not sure what the other fs does > > Thanks, > > -liubo > -- gpg @keyserver.linux.it: Goffredo Baroncelli <kreijackATinwind.it> Key fingerprint BBF5 1610 0B64 DAC6 5F7D 17B2 0EDA 9B37 8B82 E0B5 -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
