On Mon, Feb 21, 2022 at 05:48:17PM +0100, Stefano Garzarella wrote: > On Mon, Feb 21, 2022 at 12:58:51PM +0530, Anirudh Rayabharam wrote: > > Return early when userspace sends zero regions in the VHOST_SET_MEM_TABLE > > ioctl. > > > > Otherwise, this causes an erroneous entry to be added to the iotlb. This > > entry has a range size of 0 (due to u64 overflow). This then causes > > iotlb_access_ok() to loop indefinitely resulting in a hung thread. > > Syzbot has reported this here: > > > > https://syzkaller.appspot.com/bug?extid=0abd373e2e50d704db87 > > IIUC vhost_iotlb_add_range() in the for loop is never called if mem.nregions > is 0, so I'm not sure the problem reported by syzbot is related. Oh you're right. My bad! The problem is actually elsewhere. > > In any case maybe this patch is fine, but currently I think we're just > registering an iotlb without any regions, which in theory shouldn't cause > any problems. Yeah, there shouldn't be any problems here. The problem actually seems to be in vhost_process_iotlb_msg() where we end up adding a zero sized region to the iotlb. I will send a new patch... Thanks! - Anirudh. > > Thanks, > Stefano > > > > > Reported-and-tested-by: syzbot+0abd373e2e50d704db87@xxxxxxxxxxxxxxxxxxxxxxxxx > > Signed-off-by: Anirudh Rayabharam <mail@xxxxxxxxxxxxx> > > --- > > drivers/vhost/vhost.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c > > index 59edb5a1ffe2..821aba60eac2 100644 > > --- a/drivers/vhost/vhost.c > > +++ b/drivers/vhost/vhost.c > > @@ -1428,6 +1428,8 @@ static long vhost_set_memory(struct vhost_dev *d, struct vhost_memory __user *m) > > return -EFAULT; > > if (mem.padding) > > return -EOPNOTSUPP; > > + if (mem.nregions == 0) > > + return 0; > > if (mem.nregions > max_mem_regions) > > return -E2BIG; > > newmem = kvzalloc(struct_size(newmem, regions, mem.nregions), > > -- > > 2.35.1 > > >
