[pve-devel] [PATCH qemu-server 3/7] PVE/API2/Qemu: add permission checks for mapped usb devices

Dominik Csapak d.csapak at proxmox.com
Tue Aug 9 09:32:41 CEST 2022


On 8/1/22 15:01, Fabian Grünbichler wrote:
> On July 19, 2022 1:46 pm, Dominik Csapak wrote:
>> Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
>> ---
>>   PVE/API2/Qemu.pm | 39 ++++++++++++++++++++++++++++++++++++---
>>   1 file changed, 36 insertions(+), 3 deletions(-)
>>
>> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
>> index 99b426e..aa7ddea 100644
>> --- a/PVE/API2/Qemu.pm
>> +++ b/PVE/API2/Qemu.pm
>> @@ -26,6 +26,7 @@ use PVE::QemuServer::Drive;
>>   use PVE::QemuServer::ImportDisk;
>>   use PVE::QemuServer::Monitor qw(mon_cmd);
>>   use PVE::QemuServer::Machine;
>> +use PVE::QemuServer::USB qw(parse_usb_device);
>>   use PVE::QemuMigrate;
>>   use PVE::RPCEnvironment;
>>   use PVE::AccessControl;
>> @@ -567,8 +568,12 @@ my $check_vm_create_usb_perm = sub {
>>   
>>       foreach my $opt (keys %{$param}) {
>>   	next if $opt !~ m/^usb\d+$/;
>> +	my $device = parse_usb_device($param->{$opt});
>>   
>> -	if ($param->{$opt} =~ m/spice/) {
>> +	if ($device->{spice}) {
>> +	    $rpcenv->check_vm_perm($authuser, $vmid, $pool, ['VM.Config.HWType']);
>> +	} elsif ($device->{mapped}) {
>> +	    $rpcenv->check_hw_perm($authuser, $device->{host}, ['Hardware.Use']);
> 
> maybe I am overlooking something, but where does $device->{host} come
> from?
> 
> parse_usb_device (for a mapped USB device) looks up device in the
> hardware map, asserts it's valid (for the local node), and then either
> returns
> 
> {
>    vendorid => $map->{vendor},
>    productid => $map->{device},
>    mapped => 1,
> }
> 
> or the result of parse_usb_device($map->{path}), with 'mapped' set.
> 
> since the lookup in the map doesn't set a 'host' member, wouldn't
> $device->{host} always be undef for mapped devices? maybe this was
> wrongly copied from the PCI code, where the hostpci property string has
> a 'host' property (that with this series, also possibly contains a
> mapping entry ID)? or is this supposed to parse the property string, and
> use the host property from there?
> 

ok, either i did send from the wrong branch, or i redid that already since sending
the patches. my branch here locally already has all of the wrong 'parse_usb_device'
calls replaced with 'parse_property_string' (like with pci)

so in any case that is the correct approach here.
first parse the property string, then parse the usb device from the 'host' property




More information about the pve-devel mailing list