NACK/Cmnt: [PATCH] f2fs: fix a dead loop in f2fs_fiemap() Content-Type:

[Juerg Haefliger]

On Wed, 25 Jul 2018 17:39:27 +0200
Stefan Bader <stefan.bader at canonical.com> wrote:

> On 19.07.2018 18:35, Colin King wrote:
> > From: Colin Ian King <colin.king at canonical.com>
> > 
> > CVE-2017-18257
> > 
> > A dead loop can be triggered in f2fs_fiemap() using the test case
> > as below:
> > 
> >         ...
> >         fd = open();
> >         fallocate(fd, 0, 0, 4294967296);
> >         ioctl(fd, FS_IOC_FIEMAP, fiemap_buf);
> >         ...
> > 
> > It's caused by an overflow in __get_data_block():
> >         ...
> >         bh->b_size = map.m_len << inode->i_blkbits;
> >         ...
> > map.m_len is an unsigned int, and bh->b_size is a size_t which is
> > 64 bits on 64 bits archtecture, type conversion from an unsigned
> > int to a size_t will result in an overflow.
> > 
> > In the above-mentioned case, bh->b_size will be zero, and
> > f2fs_fiemap() will call get_data_block() at block 0 again an again.
> > 
> > Fix this by adding a force conversion before left shift.
> > 
> > Signed-off-by: Wei Fang <fangwei1 at huawei.com>
> > Acked-by: Chao Yu <yuchao0 at huawei.com>
> > Signed-off-by: Jaegeuk Kim <jaegeuk at kernel.org>
> > (backported from upstream commit
> > b86e33075ed1909d8002745b56ecf73b833db143) Signed-off-by: Colin Ian
> > King <colin.king at canonical.com> ---  
> 
> I think that Trusty actually is just not affected and the tracker
> needs to have hinting adjusted. fiemap looks to be only supported
> since v3.16:

OK.


> Author: Jaegeuk Kim <jaegeuk at kernel.org>
> Date:   Sun Jun 8 04:30:14 2014 +0900
> 
>     f2fs: support f2fs_fiemap
> 
> The replaced line actually has both blkbits and i as ints, so no
> conversion happens there.

But if the left-shifted value is bigger than 32bit we get an overflow
without casting. Or am I missing something?

...Juerg

 
> -Stefan
> 
> >  fs/f2fs/data.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> > index aa3438c..884bccc 100644
> > --- a/fs/f2fs/data.c
> > +++ b/fs/f2fs/data.c
> > @@ -455,7 +455,7 @@ static int get_data_block_ro(struct inode
> > *inode, sector_t iblock, != (dn.data_blkaddr + i)) || maxblocks ==
> > i) break;
> >  		map_bh(bh_result, inode->i_sb, dn.data_blkaddr);
> > -		bh_result->b_size = (i << blkbits);
> > +		bh_result->b_size = ((u64)i << blkbits);
> >  	}
> >  	f2fs_put_dnode(&dn);
> >  	trace_f2fs_get_data_block(inode, iblock, bh_result, 0);
> >   
> 
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20180726/5cc35037/attachment.sig>