Bug#674411: linux-image-2.6.32-5-xen-686: Kernel crashes if AIO is used on pages belonging to guests

[Ben Hutchings]

On Tue, 2012-06-05 at 13:01 +0100, Stefano Stabellini wrote:
> On Fri, 1 Jun 2012, Ian Campbell wrote:
> > On Thu, 2012-05-31 at 11:37 +0100, Stefano Stabellini wrote:
[...]
> > > As an alternative we could add a simple check to spot an attempt to use
> > > AIO on a granted page and return an error (still better than crashing):
[...]
> > > --- a/fs/aio.c
> > > +++ b/fs/aio.c
> > > @@ -1655,6 +1655,8 @@ SYSCALL_DEFINE3(io_submit, aio_context_t, ctx_id, 
> > > long, nr,
> > >   for (i=0; i<nr; i++) {
> > >           struct iocb __user *user_iocb;
> > >           struct iocb tmp;
> > > +         struct vm_area_struct *vma = NULL;
> > > +         struct iovec *v = NULL;
> > >  
> > >           if (unlikely(__get_user(user_iocb, iocbpp + i))) {
> > >                   ret = -EFAULT;
> > > @@ -1666,6 +1668,19 @@ SYSCALL_DEFINE3(io_submit, aio_context_t, ctx_id, 
> > > long, nr,
> > >                   break;
> > >           }
> > >  
> > > +         down_read(&current->mm->mmap_sem);
> > > +         v = (struct iovec *) tmp.aio_buf;
> > > +         /* just checking the first iovec is enough for now */
> > > +         if (v != NULL)
> > > +                 vma = find_vma(current->mm, (unsigned long) 
> > > v->iov_base);
> > > +         if (vma != NULL && vma->vm_flags & (VM_FOREIGN|VM_DONTEXPAND)) {
> > 
> > Do you mean "flags & (FOREIGN|DONTEXPACT) == (FOREIGN|DONTEXPAND)" or is
> > either one being set a bad thing?
> 
> You are correct, it should be:
> 
> if (vma != NULL && vma->vm_flags & (VM_FOREIGN|VM_DONTEXPAND) ==
>     (VM_FOREIGN|VM_DONTEXPAND)) {

'==' has higher precedence than '&', so:

if (vma != NULL && (vma->vm_flags & (VM_FOREIGN|VM_DONTEXPAND)) ==
    (VM_FOREIGN|VM_DONTEXPAND))

Ben.

-- 
Ben Hutchings
It is easier to write an incorrect program than to understand a correct one.

signature.asc
Description: This is a digitally signed message part