I caught a bug in my own code where I forgot to cast to loff_t before shifting. So I thought I'd grep around and see if I could find any other occurrences. I found a few that were clearly bugs, and they're fixed below. There are other places where we don't cast, and I think they're OK. For example, some places we have a 'nr_pages' being shifted by PAGE_SHIFT, and that's probably OK because it's probably a single I/O. Also, I didn't touch AFFS or ROMFS or some other filesystems which probably have never seen a 4GB file in their lives. Might be worth fixing to be sure nobody copies bad code from them, but not worth cc'ing stable for. I didn't look for SECTOR_SHIFT or SECTOR_SIZE (or bare 9/512), just PAGE_SIZE and PAGE_SHIFT. I can't find a GCC warning to enable for this pattern, so I filed https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97287 Matthew Wilcox (Oracle) (7): 9P: Cast to loff_t before multiplying buffer: Promote to unsigned long long before shifting ceph: Promote to unsigned long long before shifting ocfs2: Promote to unsigned long long before shifting btrfs: Promote to unsigned long long before shifting btrfs: Promote to unsigned long long before shifting btrfs: Promote to unsigned long long before multiplying fs/9p/vfs_file.c | 4 ++-- fs/btrfs/ioctl.c | 6 +++--- fs/btrfs/raid56.c | 2 +- fs/btrfs/scrub.c | 25 ++++++++++++++++--------- fs/buffer.c | 2 +- fs/ceph/addr.c | 2 +- fs/ocfs2/alloc.c | 2 +- 7 files changed, 25 insertions(+), 18 deletions(-) -- 2.28.0
On 32-bit systems, this multiplication will overflow for files larger than 4GB. Cc: stable@vger.kernel.org Fixes: fb89b45cdfdc ("9P: introduction of a new cache=mmap model.") Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> --- fs/9p/vfs_file.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/9p/vfs_file.c b/fs/9p/vfs_file.c index 3576123d8299..6d97b6b4d34b 100644 --- a/fs/9p/vfs_file.c +++ b/fs/9p/vfs_file.c @@ -612,9 +612,9 @@ static void v9fs_mmap_vm_close(struct vm_area_struct *vma) struct writeback_control wbc = { .nr_to_write = LONG_MAX, .sync_mode = WB_SYNC_ALL, - .range_start = vma->vm_pgoff * PAGE_SIZE, + .range_start = (loff_t)vma->vm_pgoff * PAGE_SIZE, /* absolute end, byte at end included */ - .range_end = vma->vm_pgoff * PAGE_SIZE + + .range_end = (loff_t)vma->vm_pgoff * PAGE_SIZE + (vma->vm_end - vma->vm_start - 1), }; -- 2.28.0
On 32-bit systems, this shift will overflow for files larger than 4GB. Cc: stable@vger.kernel.org Fixes: 5417169026c3 ("[FS] Implement block_page_mkwrite.") Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> --- fs/buffer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/buffer.c b/fs/buffer.c index 50bbc99e3d96..66f4765e60ee 100644 --- a/fs/buffer.c +++ b/fs/buffer.c @@ -2515,7 +2515,7 @@ int block_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf, } /* page is wholly or partially inside EOF */ - if (((page->index + 1) << PAGE_SHIFT) > size) + if (((page->index + 1ULL) << PAGE_SHIFT) > size) end = size & ~PAGE_MASK; else end = PAGE_SIZE; -- 2.28.0
On 32-bit systems, this shift will overflow for files larger than 4GB. Cc: stable@vger.kernel.org Fixes: 61f68816211e ("ceph: check caps in filemap_fault and page_mkwrite") Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> --- fs/ceph/addr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c index 6ea761c84494..970e5a094035 100644 --- a/fs/ceph/addr.c +++ b/fs/ceph/addr.c @@ -1522,7 +1522,7 @@ static vm_fault_t ceph_filemap_fault(struct vm_fault *vmf) struct ceph_inode_info *ci = ceph_inode(inode); struct ceph_file_info *fi = vma->vm_file->private_data; struct page *pinned_page = NULL; - loff_t off = vmf->pgoff << PAGE_SHIFT; + loff_t off = (loff_t)vmf->pgoff << PAGE_SHIFT; int want, got, err; sigset_t oldset; vm_fault_t ret = VM_FAULT_SIGBUS; -- 2.28.0
On 32-bit systems, this shift will overflow for files larger than 4GB. Cc: stable@vger.kernel.org Fixes: 35edec1d52c0 ("ocfs2: update truncate handling of partial clusters") Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> --- fs/ocfs2/alloc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/ocfs2/alloc.c b/fs/ocfs2/alloc.c index 4c1b90442d6f..26eff79ecb50 100644 --- a/fs/ocfs2/alloc.c +++ b/fs/ocfs2/alloc.c @@ -6867,7 +6867,7 @@ static void ocfs2_zero_cluster_pages(struct inode *inode, loff_t start, ocfs2_map_and_dirty_page(inode, handle, from, to, page, 1, &phys); - start = (page->index + 1) << PAGE_SHIFT; + start = (page->index + 1ULL) << PAGE_SHIFT; } out: if (pages) -- 2.28.0
On 32-bit systems, this shift will overflow for files larger than 4GB. Cc: stable@vger.kernel.org Fixes: df480633b891 ("btrfs: extent-tree: Switch to new delalloc space reserve and release") Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> --- fs/btrfs/ioctl.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index ac45f022b495..4d3b7e4ae53a 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -1277,7 +1277,7 @@ static int cluster_pages_for_defrag(struct inode *inode, page_cnt = min_t(u64, (u64)num_pages, (u64)file_end - start_index + 1); ret = btrfs_delalloc_reserve_space(BTRFS_I(inode), &data_reserved, - start_index << PAGE_SHIFT, + (loff_t)start_index << PAGE_SHIFT, page_cnt << PAGE_SHIFT); if (ret) return ret; @@ -1367,7 +1367,7 @@ static int cluster_pages_for_defrag(struct inode *inode, btrfs_mod_outstanding_extents(BTRFS_I(inode), 1); spin_unlock(&BTRFS_I(inode)->lock); btrfs_delalloc_release_space(BTRFS_I(inode), data_reserved, - start_index << PAGE_SHIFT, + (loff_t)start_index << PAGE_SHIFT, (page_cnt - i_done) << PAGE_SHIFT, true); } @@ -1395,7 +1395,7 @@ static int cluster_pages_for_defrag(struct inode *inode, put_page(pages[i]); } btrfs_delalloc_release_space(BTRFS_I(inode), data_reserved, - start_index << PAGE_SHIFT, + (loff_t)start_index << PAGE_SHIFT, page_cnt << PAGE_SHIFT, true); btrfs_delalloc_release_extents(BTRFS_I(inode), page_cnt << PAGE_SHIFT); extent_changeset_free(data_reserved); -- 2.28.0
On 32-bit systems, this shift will overflow for files larger than 4GB. Cc: stable@vger.kernel.org Fixes: 53b381b3abeb ("Btrfs: RAID5 and RAID6") Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> --- fs/btrfs/raid56.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/btrfs/raid56.c b/fs/btrfs/raid56.c index 255490f42b5d..5ee0a53301bd 100644 --- a/fs/btrfs/raid56.c +++ b/fs/btrfs/raid56.c @@ -1089,7 +1089,7 @@ static int rbio_add_io_page(struct btrfs_raid_bio *rbio, u64 disk_start; stripe = &rbio->bbio->stripes[stripe_nr]; - disk_start = stripe->physical + (page_index << PAGE_SHIFT); + disk_start = stripe->physical + ((loff_t)page_index << PAGE_SHIFT); /* if the device is missing, just fail this stripe */ if (!stripe->dev->bdev) -- 2.28.0
On 32-bit systems, these shifts will overflow for files larger than 4GB. Add helper functions to avoid this problem coming back. Cc: stable@vger.kernel.org Fixes: 73ff61dbe5ed ("Btrfs: fix device replace of a missing RAID 5/6 device") Fixes: be50a8ddaae1 ("Btrfs: Simplify scrub_setup_recheck_block()'s argument") Fixes: ff023aac3119 ("Btrfs: add code to scrub to copy read data to another disk") Fixes: b5d67f64f9bc ("Btrfs: change scrub to support big blocks") Fixes: a2de733c78fa ("btrfs: scrub") Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> --- fs/btrfs/scrub.c | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c index 354ab9985a34..ccbaf9c6e87a 100644 --- a/fs/btrfs/scrub.c +++ b/fs/btrfs/scrub.c @@ -1262,12 +1262,17 @@ static inline void scrub_stripe_index_and_offset(u64 logical, u64 map_type, } } +static u64 sblock_length(struct scrub_block *sblock) +{ + return (u64)sblock->page_count * PAGE_SIZE; +} + static int scrub_setup_recheck_block(struct scrub_block *original_sblock, struct scrub_block *sblocks_for_recheck) { struct scrub_ctx *sctx = original_sblock->sctx; struct btrfs_fs_info *fs_info = sctx->fs_info; - u64 length = original_sblock->page_count * PAGE_SIZE; + u64 length = sblock_length(original_sblock); u64 logical = original_sblock->pagev[0]->logical; u64 generation = original_sblock->pagev[0]->generation; u64 flags = original_sblock->pagev[0]->flags; @@ -1610,6 +1615,11 @@ static void scrub_write_block_to_dev_replace(struct scrub_block *sblock) } } +static u64 sbio_length(struct scrub_bio *sbio) +{ + return (u64)sbio->page_count * PAGE_SIZE; +} + static int scrub_write_page_to_dev_replace(struct scrub_block *sblock, int page_num) { @@ -1659,10 +1669,9 @@ static int scrub_add_page_to_wr_bio(struct scrub_ctx *sctx, bio->bi_iter.bi_sector = sbio->physical >> 9; bio->bi_opf = REQ_OP_WRITE; sbio->status = 0; - } else if (sbio->physical + sbio->page_count * PAGE_SIZE != + } else if (sbio->physical + sbio_length(sbio) != spage->physical_for_dev_replace || - sbio->logical + sbio->page_count * PAGE_SIZE != - spage->logical) { + sbio->logical + sbio_length(sbio) != spage->logical) { scrub_wr_submit(sctx); goto again; } @@ -2005,10 +2014,8 @@ static int scrub_add_page_to_rd_bio(struct scrub_ctx *sctx, bio->bi_iter.bi_sector = sbio->physical >> 9; bio->bi_opf = REQ_OP_READ; sbio->status = 0; - } else if (sbio->physical + sbio->page_count * PAGE_SIZE != - spage->physical || - sbio->logical + sbio->page_count * PAGE_SIZE != - spage->logical || + } else if (sbio->physical + sbio_length(sbio) != spage->physical || + sbio->logical + sbio_length(sbio) != spage->logical || sbio->dev != spage->dev) { scrub_submit(sctx); goto again; @@ -2094,7 +2101,7 @@ static void scrub_missing_raid56_pages(struct scrub_block *sblock) { struct scrub_ctx *sctx = sblock->sctx; struct btrfs_fs_info *fs_info = sctx->fs_info; - u64 length = sblock->page_count * PAGE_SIZE; + u64 length = sblock_length(sblock); u64 logical = sblock->pagev[0]->logical; struct btrfs_bio *bbio = NULL; struct bio *bio; -- 2.28.0
On Sun, 2020-10-04 at 19:04 +0100, Matthew Wilcox (Oracle) wrote:
> On 32-bit systems, this shift will overflow for files larger than 4GB.
>
> Cc: stable@vger.kernel.org
> Fixes: 61f68816211e ("ceph: check caps in filemap_fault and page_mkwrite")
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
> fs/ceph/addr.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c
> index 6ea761c84494..970e5a094035 100644
> --- a/fs/ceph/addr.c
> +++ b/fs/ceph/addr.c
> @@ -1522,7 +1522,7 @@ static vm_fault_t ceph_filemap_fault(struct vm_fault *vmf)
> struct ceph_inode_info *ci = ceph_inode(inode);
> struct ceph_file_info *fi = vma->vm_file->private_data;
> struct page *pinned_page = NULL;
> - loff_t off = vmf->pgoff << PAGE_SHIFT;
> + loff_t off = (loff_t)vmf->pgoff << PAGE_SHIFT;
> int want, got, err;
> sigset_t oldset;
> vm_fault_t ret = VM_FAULT_SIGBUS;
Good catch! Would you like us to take this in via the ceph tree, or are
you planning to submit altogether upstream? Either way:
Reviewed-by: Jeff Layton <jlayton@kernel.org>
On Sun, 2020-10-04 at 19:04 +0100, Matthew Wilcox (Oracle) wrote:
> On 32-bit systems, this shift will overflow for files larger than 4GB.
>
> Cc: stable@vger.kernel.org
> Fixes: 61f68816211e ("ceph: check caps in filemap_fault and page_mkwrite")
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
> fs/ceph/addr.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c
> index 6ea761c84494..970e5a094035 100644
> --- a/fs/ceph/addr.c
> +++ b/fs/ceph/addr.c
> @@ -1522,7 +1522,7 @@ static vm_fault_t ceph_filemap_fault(struct vm_fault *vmf)
> struct ceph_inode_info *ci = ceph_inode(inode);
> struct ceph_file_info *fi = vma->vm_file->private_data;
> struct page *pinned_page = NULL;
> - loff_t off = vmf->pgoff << PAGE_SHIFT;
> + loff_t off = (loff_t)vmf->pgoff << PAGE_SHIFT;
> int want, got, err;
> sigset_t oldset;
> vm_fault_t ret = VM_FAULT_SIGBUS;
I went ahead and merged this into the ceph-client/testing branch. Given
how old this bug is, I don't see a real need to rush this into v5.9, but
if we have any other patches going in before that ships, then it might
be good to send this one along too.
--
Jeff Layton <jlayton@kernel.org>
On Sun, Oct 04, 2020 at 07:04:22PM +0100, Matthew Wilcox (Oracle) wrote:
> On 32-bit systems, this multiplication will overflow for files larger
> than 4GB.
>
> Cc: stable@vger.kernel.org
> Fixes: fb89b45cdfdc ("9P: introduction of a new cache=mmap model.")
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
> fs/9p/vfs_file.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/9p/vfs_file.c b/fs/9p/vfs_file.c
> index 3576123d8299..6d97b6b4d34b 100644
> --- a/fs/9p/vfs_file.c
> +++ b/fs/9p/vfs_file.c
> @@ -612,9 +612,9 @@ static void v9fs_mmap_vm_close(struct vm_area_struct *vma)
> struct writeback_control wbc = {
> .nr_to_write = LONG_MAX,
> .sync_mode = WB_SYNC_ALL,
> - .range_start = vma->vm_pgoff * PAGE_SIZE,
> + .range_start = (loff_t)vma->vm_pgoff * PAGE_SIZE,
Given the may places where this issue shows up I think we really need
a vma_offset or similar helper for it. Much better than chasing missing
casts everywhere.
On Wed, Oct 07, 2020 at 06:48:49AM +0100, Christoph Hellwig wrote:
> > - .range_start = vma->vm_pgoff * PAGE_SIZE,
> > + .range_start = (loff_t)vma->vm_pgoff * PAGE_SIZE,
>
> Given the may places where this issue shows up I think we really need
> a vma_offset or similar helper for it. Much better than chasing missing
> casts everywhere.
Good point. I think these patches need to go in to fix the bugs in
the various stable releases, but we should definitely have a helper
for the future. Also, several of these patches are for non-VMA
pgoff_t.
vma_offset() is a bit weird for me -- vmas have all kinds of offsets.
vma_file_offset() would work or vma_fpos(). I tend to prefer the shorter
function name ;-)
A quick grep shows we probably want a vmf_fpos() too:
arch/powerpc/platforms/cell/spufs/file.c: unsigned long area, offset = vmf->pgoff << PAGE_SHIFT;
arch/x86/entry/vdso/vma.c: sym_offset = (long)(vmf->pgoff << PAGE_SHIFT) +
drivers/gpu/drm/gma500/framebuffer.c: address = vmf->address - (vmf->pgoff << PAGE_SHIFT);
drivers/scsi/cxlflash/ocxl_hw.c: offset = vmf->pgoff << PAGE_SHIFT;
I'm sure a lot of this will never run on a 32-bit kernel or with a 4GB
file, but it's not good to have bad code around for people to copy from.
On 10/4/20 2:04 PM, Matthew Wilcox (Oracle) wrote:
> On 32-bit systems, this shift will overflow for files larger than 4GB.
>
> Cc: stable@vger.kernel.org
> Fixes: df480633b891 ("btrfs: extent-tree: Switch to new delalloc space reserve and release")
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
Reviewed-by: Josef Bacik <josef@toxicpanda.com>
Thanks,
Josef
On 10/4/20 2:04 PM, Matthew Wilcox (Oracle) wrote:
> On 32-bit systems, this shift will overflow for files larger than 4GB.
>
> Cc: stable@vger.kernel.org
> Fixes: 53b381b3abeb ("Btrfs: RAID5 and RAID6")
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
Reviewed-by: Josef Bacik <josef@toxicpanda.com>
Thanks,
Josef
On Sun, Oct 04, 2020 at 07:04:28PM +0100, Matthew Wilcox (Oracle) wrote:
> On 32-bit systems, these shifts will overflow for files larger than 4GB.
> Add helper functions to avoid this problem coming back.
>
> Cc: stable@vger.kernel.org
> Fixes: 73ff61dbe5ed ("Btrfs: fix device replace of a missing RAID 5/6 device")
> Fixes: be50a8ddaae1 ("Btrfs: Simplify scrub_setup_recheck_block()'s argument")
> Fixes: ff023aac3119 ("Btrfs: add code to scrub to copy read data to another disk")
> Fixes: b5d67f64f9bc ("Btrfs: change scrub to support big blocks")
> Fixes: a2de733c78fa ("btrfs: scrub")
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
> fs/btrfs/scrub.c | 25 ++++++++++++++++---------
> 1 file changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
> index 354ab9985a34..ccbaf9c6e87a 100644
> --- a/fs/btrfs/scrub.c
> +++ b/fs/btrfs/scrub.c
> @@ -1262,12 +1262,17 @@ static inline void scrub_stripe_index_and_offset(u64 logical, u64 map_type,
> }
> }
>
> +static u64 sblock_length(struct scrub_block *sblock)
> +{
> + return (u64)sblock->page_count * PAGE_SIZE;
page_count will be 32 at most, the type is int and this will never
overflow. The value is usualy number of pages in the arrays scrub_bio::pagev or
scrub_block::pagev bounded by SCRUB_PAGES_PER_WR_BIO (32) or
SCRUB_MAX_PAGES_PER_BLOCK (16). The scrub code does not use mappings
and it reads raw blocks to own pages and does the checksum verification.
On Sun, Oct 04, 2020 at 07:04:27PM +0100, Matthew Wilcox (Oracle) wrote:
> On 32-bit systems, this shift will overflow for files larger than 4GB.
>
> Cc: stable@vger.kernel.org
> Fixes: 53b381b3abeb ("Btrfs: RAID5 and RAID6")
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
> fs/btrfs/raid56.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/btrfs/raid56.c b/fs/btrfs/raid56.c
> index 255490f42b5d..5ee0a53301bd 100644
> --- a/fs/btrfs/raid56.c
> +++ b/fs/btrfs/raid56.c
> @@ -1089,7 +1089,7 @@ static int rbio_add_io_page(struct btrfs_raid_bio *rbio,
> u64 disk_start;
>
> stripe = &rbio->bbio->stripes[stripe_nr];
> - disk_start = stripe->physical + (page_index << PAGE_SHIFT);
> + disk_start = stripe->physical + ((loff_t)page_index << PAGE_SHIFT);
It seems that this patch is mechanical replacement. If you check the
callers, the page_index is passed from an int that iterates over bits
set in an unsigned long (bitmap). The result won't overflow.
On Mon, Oct 26, 2020 at 05:35:46PM +0100, David Sterba wrote:
> On Sun, Oct 04, 2020 at 07:04:27PM +0100, Matthew Wilcox (Oracle) wrote:
> > On 32-bit systems, this shift will overflow for files larger than 4GB.
> >
> > Cc: stable@vger.kernel.org
> > Fixes: 53b381b3abeb ("Btrfs: RAID5 and RAID6")
> > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> > ---
> > fs/btrfs/raid56.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/fs/btrfs/raid56.c b/fs/btrfs/raid56.c
> > index 255490f42b5d..5ee0a53301bd 100644
> > --- a/fs/btrfs/raid56.c
> > +++ b/fs/btrfs/raid56.c
> > @@ -1089,7 +1089,7 @@ static int rbio_add_io_page(struct btrfs_raid_bio *rbio,
> > u64 disk_start;
> >
> > stripe = &rbio->bbio->stripes[stripe_nr];
> > - disk_start = stripe->physical + (page_index << PAGE_SHIFT);
> > + disk_start = stripe->physical + ((loff_t)page_index << PAGE_SHIFT);
>
> It seems that this patch is mechanical replacement. If you check the
> callers, the page_index is passed from an int that iterates over bits
> set in an unsigned long (bitmap). The result won't overflow.
Not mechanical, but I clearly made mistakes. Will you pick up the
patches which actually fix bugs?
On Sun, Oct 04, 2020 at 07:04:26PM +0100, Matthew Wilcox (Oracle) wrote: > On 32-bit systems, this shift will overflow for files larger than 4GB. > > Cc: stable@vger.kernel.org > Fixes: df480633b891 ("btrfs: extent-tree: Switch to new delalloc space reserve and release") > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> > --- > fs/btrfs/ioctl.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c > index ac45f022b495..4d3b7e4ae53a 100644 > --- a/fs/btrfs/ioctl.c > +++ b/fs/btrfs/ioctl.c > @@ -1277,7 +1277,7 @@ static int cluster_pages_for_defrag(struct inode *inode, > page_cnt = min_t(u64, (u64)num_pages, (u64)file_end - start_index + 1); > > ret = btrfs_delalloc_reserve_space(BTRFS_I(inode), &data_reserved, > - start_index << PAGE_SHIFT, > + (loff_t)start_index << PAGE_SHIFT, > - start_index << PAGE_SHIFT, > + (loff_t)start_index << PAGE_SHIFT, > - start_index << PAGE_SHIFT, > + (loff_t)start_index << PAGE_SHIFT, As this repeats 3 times I've added a variable. Patch added to misc-next, thanks.
On Mon, Oct 26, 2020 at 04:44:42PM +0000, Matthew Wilcox wrote:
> On Mon, Oct 26, 2020 at 05:35:46PM +0100, David Sterba wrote:
> > On Sun, Oct 04, 2020 at 07:04:27PM +0100, Matthew Wilcox (Oracle) wrote:
> > > On 32-bit systems, this shift will overflow for files larger than 4GB.
> > >
> > > Cc: stable@vger.kernel.org
> > > Fixes: 53b381b3abeb ("Btrfs: RAID5 and RAID6")
> > > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> > > ---
> > > fs/btrfs/raid56.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/fs/btrfs/raid56.c b/fs/btrfs/raid56.c
> > > index 255490f42b5d..5ee0a53301bd 100644
> > > --- a/fs/btrfs/raid56.c
> > > +++ b/fs/btrfs/raid56.c
> > > @@ -1089,7 +1089,7 @@ static int rbio_add_io_page(struct btrfs_raid_bio *rbio,
> > > u64 disk_start;
> > >
> > > stripe = &rbio->bbio->stripes[stripe_nr];
> > > - disk_start = stripe->physical + (page_index << PAGE_SHIFT);
> > > + disk_start = stripe->physical + ((loff_t)page_index << PAGE_SHIFT);
> >
> > It seems that this patch is mechanical replacement. If you check the
> > callers, the page_index is passed from an int that iterates over bits
> > set in an unsigned long (bitmap). The result won't overflow.
>
> Not mechanical, but I clearly made mistakes. Will you pick up the
> patches which actually fix bugs?
Yes, I just replied to the first patch, that does fix an overflow.
Matthew Wilcox (Oracle) wrote on Sun, Oct 04, 2020:
> On 32-bit systems, this multiplication will overflow for files larger
> than 4GB.
>
> Cc: stable@vger.kernel.org
> Fixes: fb89b45cdfdc ("9P: introduction of a new cache=mmap model.")
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
I realize I hadn't sent a mail -- FWIW this 9p patch has been merged,
thanks!
--
Dominique