From: Elias Huhsovitz <e.huhsovitz@proxmox.com>
To: pve-devel@lists.proxmox.com
Cc: Elias Huhsovitz <e.huhsovitz@proxmox.com>
Subject: [PATCH manager v2] fix #6735: api: pci: allow mdevscan access via mapping permissions.
Date: Thu, 27 Aug 2026 13:25:25 +0200 [thread overview]
Message-ID: <20260827112525.154445-1-e.huhsovitz@proxmox.com> (raw)
The mdevscan endpoint requires Sys.Audit or Sys.Modify on '/'. This
blocks non-admin users from listing mediated device types for a PCI
mapping, even when they hold Mapping.Use on that mapping.
Check the permission in the API handler based on the parameter type: For
a raw PCI ID, require Sys.Audit or Sys.Modify on '/'. For a mapping,
require Sys.Audit or Sys.Modify on '/', or fall back to requiring
Mapping.Use, Mapping.Modify or Mapping.Audit on the specific mapping
path.
Set the endpoint permission to 'user => all' so the handler performs the
type-dependent check. This keeps raw PCI IDs, which are not valid ACL
paths, out of the declarative ACL evaluation.
Reduce indentation by removing redundant else statement. (return
provides implicit branching)
Signed-off-by: Elias Huhsovitz <e.huhsovitz@proxmox.com>
---
v1: https://lore.proxmox.com/pve-devel/20260824112610.148089-1-e.huhsovitz@proxmox.com/
Changes v1->v2
--------------
* Allow all users in the declarative API permissions.
* Implement ACL check in API handler by calling
check_any and raise_perm_exc.
* Remove else statement in API handler, since return statement
provides implicit branching
* update commit message
Side Note
---------
Since this pattern of manual permission checks appear quite frequently,
it might be of interest to provide generic way to deal with such cases
inside of the permissions block (Similar to overriding the
hasPermission method in Java Spring Boot)
For example like this:
permissions => {
user => 'all',
custom => my_perm_sub(),
},
my_perm_sub would simply return a boolean. This would be an optional
extension, without requiring major changes in ACL Logic.
Let me know if this is something you would consider worthwhile.
Changes
-------
PVE/API2/Hardware/PCI.pm | 71 +++++++++++++++++++++++++++-------------
1 file changed, 48 insertions(+), 23 deletions(-)
diff --git a/PVE/API2/Hardware/PCI.pm b/PVE/API2/Hardware/PCI.pm
index 36b9741b..47c4b29b 100644
--- a/PVE/API2/Hardware/PCI.pm
+++ b/PVE/API2/Hardware/PCI.pm
@@ -3,10 +3,12 @@ package PVE::API2::Hardware::PCI;
use strict;
use warnings;
+use PVE::Exception qw(raise raise_perm_exc);
use PVE::JSONSchema qw(get_standard_option);
use PVE::QemuServer::PCI::Mdev;
use PVE::RESTHandler;
+use PVE::RPCEnvironment;
use base qw(PVE::RESTHandler);
@@ -180,7 +182,11 @@ __PACKAGE__->register_method({
protected => 1,
proxyto => "node",
permissions => {
- check => ['perm', '/', ['Sys.Audit', 'Sys.Modify'], any => 1],
+ description =>
+ "For a PCI ID, requires 'Sys.Audit' or 'Sys.Modify' on '/'. For a mapping,"
+ . " requires the same global permissions, or 'Mapping.Use', 'Mapping.Modify'"
+ . ", or'Mapping.Audit' on '/mapping/pci/<id>'.",
+ user => 'all',
},
parameters => {
additionalProperties => 0,
@@ -222,31 +228,50 @@ __PACKAGE__->register_method({
code => sub {
my ($param) = @_;
- if ($param->{'pci-id-or-mapping'} =~
- m/^(?:[0-9a-fA-F]{4}:)?[0-9a-fA-F]{2}:[0-9a-fA-F]{2}\.[0-9a-fA-F]$/
- ) {
- return PVE::QemuServer::PCI::Mdev::get_mdev_types($param->{'pci-id-or-mapping'}); # PCI ID
- } else {
- my $mapping = $param->{'pci-id-or-mapping'};
-
- my $types = {};
- my $devices = PVE::Mapping::PCI::find_on_current_node($mapping);
- for my $device ($devices->@*) {
- my $id = $device->{path};
- next if $id =~ m/;/; # mdev not supported for multifunction devices
-
- my $device_types = PVE::QemuServer::PCI::Mdev::get_mdev_types($id);
-
- for my $type_definition ($device_types->@*) {
- my $type = $type_definition->{type};
- if (!defined($types->{$type})) {
- $types->{$type} = $type_definition;
- }
+ my $id = $param->{'pci-id-or-mapping'};
+ my $rpcenv = PVE::RPCEnvironment::get();
+ my $authuser = $rpcenv->get_user();
+
+ my $is_pci_id =
+ $id =~ m/^(?:[0-9a-fA-F]{4}:)?[0-9a-fA-F]{2}:[0-9a-fA-F]{2}\.[0-9a-fA-F]$/;
+ my $has_global_perms =
+ $rpcenv->check_any($authuser, '/', ['Sys.Audit', 'Sys.Modify'], 1);
+
+ if (!$has_global_perms) {
+ raise_perm_exc("/, " . join("|", ['Sys.Audit', 'Sys.Modify'])) if ($is_pci_id);
+ $rpcenv->check_any(
+ $authuser,
+ "/mapping/pci/$id",
+ ['Mapping.Use', 'Mapping.Modify', 'Mapping.Audit'],
+ );
+ }
+
+ return PVE::QemuServer::PCI::Mdev::get_mdev_types($id) if ($is_pci_id);
+
+ if (!$rpcenv->check_any($authuser, '/', ['Sys.Audit', 'Sys.Modify'], 1)) {
+ $rpcenv->check_any(
+ $authuser,
+ "/mapping/pci/$id",
+ ['Mapping.Use', 'Mapping.Modify', 'Mapping.Audit'],
+ );
+ }
+
+ my $types = {};
+ my $devices = PVE::Mapping::PCI::find_on_current_node($id);
+ for my $device ($devices->@*) {
+ my $dev_id = $device->{path};
+ next if $dev_id =~ m/;/; # mdev not supported for multifunction devices
+
+ my $device_types = PVE::QemuServer::PCI::Mdev::get_mdev_types($dev_id);
+
+ for my $type_definition ($device_types->@*) {
+ my $type = $type_definition->{type};
+ if (!defined($types->{$type})) {
+ $types->{$type} = $type_definition;
}
}
-
- return [sort { $a->{type} cmp $b->{type} } values($types->%*)];
}
+ return [sort { $a->{type} cmp $b->{type} } values($types->%*)];
},
});
--
2.47.3
reply other threads:[~2026-08-27 11:25 UTC|newest]
Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260827112525.154445-1-e.huhsovitz@proxmox.com \
--to=e.huhsovitz@proxmox.com \
--cc=pve-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox