Re: [Xen-devel] [PATCH 2/3] User-space grant table device - main driver

[Derek Murray <Derek.Murray@xxxxxxxxxxxx>]

On 29 Mar 2007, at 07:26, Isaku Yamahata wrote:
> On Wed, Mar 28, 2007 at 01:56:16PM +0100, Derek Murray wrote:
>
> > +static int gntdev_mmap (struct file *flip, struct vm_area_struct  
> > *vma)
> > +{
> ...
> > +undo_map_out:
> This doesn't undo vm_insert_page() when auto trasnalted physmap mode.
> Probably just calling zap_page_range() would be OK instead
> of undo_count_kmap loop.
Hmm, in fact, this whole section is redundant, because a failing  
gntdev_mmap() will be cleaned up in do_mmap_pgoff(), and,  
subsequently, the gntdev_clear_pte() hook will be called. This does a  
pte_clear_full() on the user-space PTE in auto translated physmap  
mode, so would this be enough?
> > +static pte_t gntdev_clear_pte(struct vm_area_struct *vma,  
> > unsigned long addr,
> > +                             pte_t *ptep, int is_fullmm)
> Please clear the entry before grant table unmap.
> If the virtual address is accessed between grant tabel unmap and
> pte_clear_full(), something bad would happen.
> The higher level exclusion is done and I might be somewhat  
> paranoia, though.
Better safe than sorry! I'll fix this.

> > +static int gntdev_ioctl(struct inode *inode, struct file *flip,
> > +                       unsigned int cmd, unsigned long arg)
> copy_from/to_user() may result in page fault and page fault handler
> may try to obtain it causing dead lock.
Ah, good point. I'll move these copies out of the critical section.

Thanks for your comments!

Regards,

Derek Murray.



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel