lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Fri, 21 Jun 2013 19:26:29 -0600
From:	Alex Williamson <alex.williamson@...hat.com>
To:	Alexey Kardashevskiy <aik@...abs.ru>
Cc:	linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
	Benjamin Herrenschmidt <benh@...nel.crashing.org>
Subject: Re: [PATCH] vfio: Limit group opens

On Sat, 2013-06-22 at 11:16 +1000, Alexey Kardashevskiy wrote:
> Cool, thanks!
> 
> So we will need only this (to be called from KVM), and that will be it, right?

For what?  This is not the external lock you're looking for.  As I've
mentioned, the file can only hold the group, but that doesn't give you
any guarantee that the group is protected by the IOMMU.  Thanks,

Alex

> int vfio_group_iommu_id_from_file(struct file *filep)
> ...
> 
> 
> 
> On 06/22/2013 07:12 AM, Alex Williamson wrote:
> > vfio_group_fops_open attempts to limit concurrent sessions by
> > disallowing opens once group->container is set.  This really doesn't
> > do what we want and allow for inconsistent behavior, for instance a
> > group can be opened twice, then a container set giving the user two
> > file descriptors to the group.  But then it won't allow more to be
> > opened.  There's not much reason to have the group opened multiple
> > times since most access is through devices or the container, so
> > complete what the original code intended and only allow a single
> > instance.
> > 
> > Signed-off-by: Alex Williamson <alex.williamson@...hat.com>
> > ---
> >  drivers/vfio/vfio.c |   14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> > 
> > diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> > index 6d78736..d30f44d 100644
> > --- a/drivers/vfio/vfio.c
> > +++ b/drivers/vfio/vfio.c
> > @@ -76,6 +76,7 @@ struct vfio_group {
> >  	struct notifier_block		nb;
> >  	struct list_head		vfio_next;
> >  	struct list_head		container_next;
> > +	atomic_t			opened;
> >  };
> >  
> >  struct vfio_device {
> > @@ -206,6 +207,7 @@ static struct vfio_group *vfio_create_group(struct iommu_group *iommu_group)
> >  	INIT_LIST_HEAD(&group->device_list);
> >  	mutex_init(&group->device_lock);
> >  	atomic_set(&group->container_users, 0);
> > +	atomic_set(&group->opened, 0);
> >  	group->iommu_group = iommu_group;
> >  
> >  	group->nb.notifier_call = vfio_iommu_group_notifier;
> > @@ -1236,12 +1238,22 @@ static long vfio_group_fops_compat_ioctl(struct file *filep,
> >  static int vfio_group_fops_open(struct inode *inode, struct file *filep)
> >  {
> >  	struct vfio_group *group;
> > +	int opened;
> >  
> >  	group = vfio_group_get_from_minor(iminor(inode));
> >  	if (!group)
> >  		return -ENODEV;
> >  
> > +	/* Do we need multiple instances of the group open?  Seems not. */
> > +	opened = atomic_cmpxchg(&group->opened, 0, 1);
> > +	if (opened) {
> > +		vfio_group_put(group);
> > +		return -EBUSY;
> > +	}
> > +
> > +	/* Is something still in use from a previous open? */
> >  	if (group->container) {
> > +		atomic_dec(&group->opened);
> >  		vfio_group_put(group);
> >  		return -EBUSY;
> >  	}
> > @@ -1259,6 +1271,8 @@ static int vfio_group_fops_release(struct inode *inode, struct file *filep)
> >  
> >  	vfio_group_try_dissolve_container(group);
> >  
> > +	atomic_dec(&group->opened);
> > +
> >  	vfio_group_put(group);
> >  
> >  	return 0;
> > 
> 
> 



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists