re: [PATCH] netbsd32 swapctl, round 4

[matthew green <mrg%eterna.com.au@localhost>]

> > +   int count = SCARG(uap, misc);
> > +   int i, error = 0;
> > +
> > +   /* Make sure userland cannot exhaust kernel memory */
> > +   if ((size_t)count > (size_t)uvmexp.nswapdev)
> > +           count = uvmexp.nswapdev;
> > +
> > +   sep = kmem_alloc(sizeof(*sep) * count, KM_SLEEP);
> > +   sep32 = kmem_alloc(sizeof(*sep32) * count, KM_SLEEP);
> > +
> > +   uvm_swap_stats(SWAP_STATS, sep, count, retval);
> > +   count = *retval;
> > +
> > +   if (count < 1)
> > +           goto out;
> > +
> > +   for (i = 0; i < count; i++) {
> > +           sep32[i].se_dev = sep[i].se_dev;
> > +           sep32[i].se_flags = sep[i].se_flags;
> > +           sep32[i].se_nblks = sep[i].se_nblks;
> > +           sep32[i].se_inuse = sep[i].se_inuse;
> > +           sep32[i].se_priority = sep[i].se_priority;
> > +           memcpy(sep32[i].se_path, sep[i].se_path,
> > +                   sizeof(sep32[i].se_path));
> > +   }
> 
> I don't think that nswapdev can go ridiculously high anyway, however I 
> think that the bound checks for "count" does prevent kernel memory 
> exhaustion but not corruption.
> 
> You can overflow "sizeof(*sep) * count", make the kmem_alloc(...) 
> succeed (the overflow will result in a small size_t if "count" is 
> properly chosen which is the size kmem_alloc() expects), then corrupt 
> adjacent kernel memory through the loop when writing into sep32 array.

it would require having about 4 million swap devices to trigger this.

... nothing to see here, move right along.  :-)


.mrg.