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. 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. Thanks, -liubo -- 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
