Mark Kettenis wrote: > The diff below fixes a couple of potential integer overflows in the > uvm address selection code. Most of these are in code that is > disabled, such as uaddr_lin_select and the sruff dealing with guard > pages (guard_sz/guardsz is currently always 0). But I think the > overflow in uvm_addr_fitspace() and uaddr_rnd_select() could be > reached with carefully crafted input. > > ok? > > > Index: uvm_addr.c > =================================================================== > RCS file: /cvs/src/sys/uvm/uvm_addr.c,v > retrieving revision 1.16 > diff -u -p -r1.16 uvm_addr.c > --- uvm_addr.c 2 Jun 2016 18:48:01 -0000 1.16 > +++ uvm_addr.c 29 Jul 2016 21:03:40 -0000 > @@ -254,7 +254,9 @@ uvm_addr_fitspace(vaddr_t *min_result, v > if (low_addr > high_addr) > return ENOMEM; > fspace = high_addr - low_addr; > - if (fspace < sz + before_gap + after_gap) > + if (fspace < before_gap + after_gap) > + return ENOMEM; > + if (fspace - before_gap - after_gap < sz) > return ENOMEM; > > /* Calculate lowest address. */
this looks good. I'm a little confused about the following. > @@ -520,7 +522,7 @@ uaddr_lin_select(struct vm_map *map, str > /* Deal with guardpages: search for space with one extra page. */ > guard_sz = ((map->flags & VM_MAP_GUARDPAGES) == 0 ? 0 : PAGE_SIZE); > > - if (uaddr->uaddr_maxaddr - uaddr->uaddr_minaddr < sz + guard_sz) > + if (uaddr->uaddr_maxaddr - uaddr->uaddr_minaddr - guard_sz < sz) > return ENOMEM; > return uvm_addr_linsearch(map, uaddr, entry_out, addr_out, 0, sz, > align, offset, 1, uaddr->uaddr_minaddr, uaddr->uaddr_maxaddr - sz, > @@ -582,6 +584,8 @@ uaddr_rnd_select(struct vm_map *map, str > /* Deal with guardpages: search for space with one extra page. */ > guard_sz = ((map->flags & VM_MAP_GUARDPAGES) == 0 ? 0 : PAGE_SIZE); > > + if (uaddr->uaddr_maxaddr - guard_sz < sz) > + return ENOMEM; > minaddr = uvm_addr_align_forward(uaddr->uaddr_minaddr, align, offset); > maxaddr = uvm_addr_align_backward(uaddr->uaddr_maxaddr - sz - guard_sz, > align, offset); This one especially. Shouldn't there be a minaddr check? It seems weird to compare addresses against sizes. > @@ -964,6 +968,8 @@ uaddr_bestfit_select(struct vm_map *map, > > uaddr = (struct uaddr_bestfit_state *)uaddr_p; > guardsz = ((map->flags & VM_MAP_GUARDPAGES) ? PAGE_SIZE : 0); > + if (sz + guardsz < sz) > + return ENOMEM; > > /* > * Find smallest item on freelist capable of holding item. >
