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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <cba9e18a-3add-4fd1-89ad-bb5d0fc521e4@intel.com>
Date: Fri, 28 Jun 2024 17:48:11 +0800
From: Yi Liu <yi.l.liu@...el.com>
To: Yan Zhao <yan.y.zhao@...el.com>, Jason Gunthorpe <jgg@...dia.com>
CC: "Tian, Kevin" <kevin.tian@...el.com>, "kvm@...r.kernel.org"
	<kvm@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "alex.williamson@...hat.com"
	<alex.williamson@...hat.com>, "peterx@...hat.com" <peterx@...hat.com>,
	"ajones@...tanamicro.com" <ajones@...tanamicro.com>
Subject: Re: [PATCH] vfio: Reuse file f_inode as vfio device inode

On 2024/6/28 13:21, Yan Zhao wrote:
> On Thu, Jun 27, 2024 at 09:42:09AM -0300, Jason Gunthorpe wrote:
>> On Thu, Jun 27, 2024 at 05:51:01PM +0800, Yan Zhao wrote:
>>
>>>>>> This doesn't seem right.. There is only one device but multiple file
>>>>>> can be opened on that device.
>>> Maybe we can put this assignment to vfio_df_ioctl_bind_iommufd() after
>>> vfio_df_open() makes sure device->open_count is 1.
>>
>> Yeah, that seems better.
>>
>> Logically it would be best if all places set the inode once the
>> inode/FD has been made to be the one and only way to access it.
> For group path, I'm afraid there's no such a place ensuring only one active fd
> in kernel.
> I tried modifying QEMU to allow two openings and two assignments of the same
> device. It works and appears to guest that there were 2 devices, though this
> ultimately leads to device malfunctions in guest.
> 
>>> BTW, in group path, what's the benefit of allowing multiple open of device?
>>
>> I don't know, the thing that opened the first FD can just dup it, no
>> idea why two different FDs would be useful. It is something we removed
>> in the cdev flow
>>
> Thanks. However, from the code, it reads like a drawback of the cdev flow :)
> I don't understand why the group path is secure though.
> 
>          /*
>           * Only the group path allows the device to be opened multiple
>           * times.  The device cdev path doesn't have a secure way for it.
>           */
>          if (device->open_count != 0 && !df->group)
>                  return -EINVAL;
> 
> 

The group path only allow single group open, so the device FDs retrieved
via the group is just within the opener of the group. This secure is built
on top of single open of group.

-- 
Regards,
Yi Liu

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ