Re: [PATCH 4/9] fibmap: Use bmap instead of ->bmap method in ioctl_fibmap

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Aug 02, 2019 at 11:19:39AM +0200, Carlos Maiolino wrote:
> Hi Darrick.
> 
> > > +		return error;
> > > +
> > > +	block = ur_block;
> > > +	error = bmap(inode, &block);
> > > +
> > > +	if (error)
> > > +		ur_block = 0;
> > > +	else
> > > +		ur_block = block;
> > 
> > What happens if ur_block > INT_MAX?  Shouldn't we return zero (i.e.
> > error) instead of truncating the value?  Maybe the code does this
> > somewhere else?  Here seemed like the obvious place for an overflow
> > check as we go from sector_t to int.
> > 
> 
> The behavior should still be the same. It will get truncated, unfortunately. I
> don't think we can actually change this behavior and return zero instead of
> truncating it.

But that's even worse, because the programs that rely on FIBMAP will now
receive *incorrect* results that may point at a different file and
definitely do not point at the correct file block.

Note also that the iomap (and therefore xfs) implementation WARNs on
integer overflow and returns 0 (error) to prevent an incorrect access.

--D

> > --D
> > 
> > > +
> > > +	error = put_user(ur_block, p);
> > > +
> > > +	return error;
> > >  }
> > >  
> > >  /**
> > > -- 
> > > 2.20.1
> > > 
> 
> -- 
> Carlos



[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux