Re: [PATCH v3 1/2] x86/domctl: don't imply I/O port permissions from I/O port mapping

[Jan Beulich <jbeulich@xxxxxxxx>]

On 21.07.2026 16:49, Roger Pau Monné wrote:
> On Tue, Jun 30, 2026 at 03:54:04PM +0200, Jan Beulich wrote:
>> Rather than granting permissions when mapping (an operation that DM-s are
>> allowed to carry out, while they can't invoke ioport-permission), check
>> whether permissions actually were granted when adding a mapping. This then
>> also allows relaxing the necessary locking.
>>
>> While no longer granting permissions upon mapping is "only" at risk of
>> breaking guests, no longer revoking permissions upon unmapping strictly
>> requires callers to additionally invoke XEN_DOMCTL_ioport_permission. Or
>> else a security issue would arise. In-tree code already does so.
>>
>> While there switch to using %pd in the two log messages.
>>
>> Fixes: 192c4dabc344 ("domctl and p2m changes for PCI passthru")
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> It's difficult to not think about someone appearing in 3 or 4 years
> complaining that this change in behavior has caused them a security
> issue, but I do agree that the previous behavior was inconsistent at
> best:

A positive aspect here is that the similar change for MMIO went without
any fallout, as far as I'm aware.

> Acked-by: Roger Pau Monné <roger@xxxxxxxxxxxxxx>

Thanks, but ...

> However you need to bump XEN_DOMCTL_INTERFACE_VERSION to note the ABI
> change as we are in 4.23 now.

... there's no interface change here. A bump therefore won't buy us
anything. I'm hesitant to take your ack as long as we disagree here.

> We possibly need to sort out the questions below, in case there are
> changes required to either libxl or QEMU.

Indeed.

>> ---
>> libxl has libxl__grant_vga_iomem_permission(), but I can't spot any I/O
>> port equivalent (nor a revoke counterpart, btw). Everywhere else MMIO and
>> I/O ports look to be treated equally.
>>
>> Qemu uses both xc_domain_{iomem_permission,memory_mapping}() in
>> igd_write_opregion(), but only xc_domain_{memory,ioport}_mapping() in
>> xen_pt_region_update() and xen_pt_{,un}register_vga_regions(). Is the IGD
>> region special in any way? Clearly this can't work from a stubdom.
> 
> Those possibly need to be answered by Anthony, but he isn't on Cc?

Oh, I had him Cc-ed on v2, but forgot to on v3. Anthony, your input is much
appreciated.

Jan