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

[Roger Pau Monné <roger@xxxxxxxxxxxxxx>]

On Tue, Jul 21, 2026 at 05:36:37PM +0200, Jan Beulich wrote:
> On 21.07.2026 17:34, Jan Beulich wrote:
> > On 21.07.2026 17:07, Roger Pau Monné wrote:
> >> On Tue, Jul 21, 2026 at 04:57:36PM +0200, Jan Beulich wrote:
> >>> 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.
> >>
> >> It's not an interface change, but it's a change in behavior of an
> >> existing hypercall in a security-relevant way, so people would
> >> generally need to be aware of it, just in case.
> > 
> > But the interface version bump won't necessarily make anyone aware. Things
> > will continue to build as before, and there's also no binary 
> > incompatibility.
> > The best way of making people aware is the ChangeLog entry that iirc you had
> > asked for (and that's now there).
> 
> I should probably add that bumping the interface version now also wouldn't do
> any harm; there's at least one other series pending which wants to do this
> anyway in this starting release cycle. Yet I still think the bumping doesn't
> quite belong here.

I was going to say - it's very likely the interface will be bumped
anyway.  I'm not going to insist, the text in domctl.h does mention
that the interface should only be bumped "in case the interface is
modified in an incompatible way", and you might argue there's no
interface change here at all, as the interface stays the same, what
changes is the underlying implementation.

Thanks, Roger.