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

From: Carlos Maiolino <cmaiolino@redhat.com>
To: "Darrick J. Wong" <darrick.wong@oracle.com>
Cc: linux-fsdevel@vger.kernel.org, hch@lst.de, adilger@dilger.ca,
	jaegeuk@kernel.org, miklos@szeredi.hu, rpeterso@redhat.com,
	linux-xfs@vger.kernel.org
Subject: Re: [PATCH 4/9] fibmap: Use bmap instead of ->bmap method in ioctl_fibmap
Date: Fri, 2 Aug 2019 11:19:39 +0200	[thread overview]
Message-ID: <20190802091937.kwutqtwt64q5hzkz@pegasus.maiolino.io> (raw)
In-Reply-To: <20190731231217.GV1561054@magnolia>

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.

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

-- 
Carlos