On Wed, May 08, 2024 at 11:29:15AM +1000, Dave Chinner wrote: > On Sat, May 04, 2024 at 02:27:36PM +0300, Dan Carpenter wrote: > > The fxr->file1_offset and fxr->file2_offset variables come from the user > > in xfs_ioc_exchange_range(). They are size loff_t which is an s64. > > Check the they aren't negative. > > > > Fixes: 9a64d9b3109d ("xfs: introduce new file range exchange ioctl") > > Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx> > > --- > > From static analysis. Untested. Sorry! > > > > fs/xfs/xfs_exchrange.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/fs/xfs/xfs_exchrange.c b/fs/xfs/xfs_exchrange.c > > index c8a655c92c92..3465e152d928 100644 > > --- a/fs/xfs/xfs_exchrange.c > > +++ b/fs/xfs/xfs_exchrange.c > > @@ -337,6 +337,9 @@ xfs_exchange_range_checks( > > if (IS_SWAPFILE(inode1) || IS_SWAPFILE(inode2)) > > return -ETXTBSY; > > > > + if (fxr->file1_offset < 0 || fxr->file2_offset < 0) > > + return -EINVAL; > > Aren't the operational offset/lengths already checked for underflow > and overflow via xfs_exchange_range_verify_area()? Oh, yeah, they are. I was just thinking surely I wrote some tests to pass in garbage offsets and bounce back out... --D > -Dave. > -- > Dave Chinner > david@xxxxxxxxxxxxx