On Fri, Aug 19, 2016 at 4:01 PM, Barret Rhoden <[email protected]> wrote:
> That should clearly be a +, not a -, since we're figuring out how far into > the VMR to map. > > This would trigger if you had a file mmapped that wasn't MAP_POPULATE, then > had a uthread fault on accessing that file, and it wasn't on the first page > of the file. > > While we're here, we can also catch any integer overflows with offset and > length. > > Signed-off-by: Barret Rhoden <[email protected]> > --- > kern/src/mm.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/kern/src/mm.c b/kern/src/mm.c > index 1202f0f52e15..f05e33e664f6 100644 > --- a/kern/src/mm.c > +++ b/kern/src/mm.c > @@ -448,6 +448,11 @@ void *mmap(struct proc *p, uintptr_t addr, size_t > len, int prot, int flags, > return MAP_FAILED; > } > } > + /* Check for overflow. This helps do_mmap and populate_va, among > others. */ > + if (offset + len < offset) { > I would be slightly worried about the optimizer getting clever and deciding this "can never happen" and removing the check. A safer method may be to change this to, 'if (SIZE_MAX - len < offset)' using either the appropriate constant from inttypes.h or a symbol set to ~(size_t)0ULL. Otherwise, LGTM. + set_errno(EINVAL); > + return MAP_FAILED; > + } > /* If they don't care where to put it, we'll start looking after > the break. > * We could just have userspace handle this (in glibc's mmap), so > we don't > * need to know about BRK_END, but this will work for now (and may > avoid > @@ -1128,9 +1133,11 @@ unsigned long populate_va(struct proc *p, uintptr_t > va, unsigned long nr_pgs) > } else { > /* need to keep the file alive in case we > unlock/block */ > kref_get(&vmr->vm_file->f_kref, 1); > + /* Regarding foff + (va - base): va - base < len, > and foff + len > + * does not over flow */ > ret = populate_pm_va(p, va, nr_pgs_this_vmr, > pte_prot, > vmr->vm_file->f_mapping, > - vmr->vm_foff - (va - > vmr->vm_base), > + vmr->vm_foff + (va - > vmr->vm_base), > vmr->vm_flags, vmr->vm_prot & > PROT_EXEC); > kref_put(&vmr->vm_file->f_kref); > if (ret) { > -- > 2.8.0.rc3.226.g39d4020 > > -- > You received this message because you are subscribed to the Google Groups > "Akaros" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to [email protected]. > To post to this group, send email to [email protected]. > For more options, visit https://groups.google.com/d/optout. > -- You received this message because you are subscribed to the Google Groups "Akaros" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected]. For more options, visit https://groups.google.com/d/optout.
