Re: another possible integer truncation in xfs

["Darrick J. Wong" <darrick.wong@oracle.com>]

On Mon, Aug 21, 2017 at 10:16:03AM +0200, Christoph Hellwig wrote:
> On Mon, Aug 21, 2017 at 08:01:03AM +0000, Markus Stockhausen wrote:
> > Hi Christoph,
> > 
> > out of curiosity I looked for other use cases of min_t in xfs. At least 
> > until 4.12 there is a similar constellation in xfs_dir2_leaf_readbuf:
> > 
> >   if (trim_map) {
> >     mip->map_blocks -= geo->fsbcount;
> >     /*
> >      * Loop to get rid of the extents for the
> >      * directory block.
> >      */
> >     for (i = geo->fsbcount; i > 0; ) {
> >       j = min_t(int, map->br_blockcount, i);
> >       map->br_blockcount -= j;
> >       map->br_startblock += j;
> >       map->br_startoff += j;
> > 
> > The loop could go havoc if map->br_blockcount is larger than 
> > 2G. If you think it could classify for stable feel free to add it too.

"2G"... are you concerned about an integer overflow if map->br_blockcount
is a value larger than 2147483647, or if *map itself represents an
extent longer than 2GiB?  (I'm pretty sure you're talking about the
first scenario, but the units here are ambiguous.)

I /think/ the correct answer here is that file extent records can't ever
be longer than 2^20 blocks so this min_t ought to be fine.

--D

> I don't think it has a chance to be larger in practice, but we should
> fix it anyway.  I'll prepare a patch.
> 
> Thanks for spotting this!
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html