| Message ID | c976d480399a44e09b1da3ad201e3021def223f7.camel@infradead.org |
|---|---|
| State | New |
| Headers | show |
| Series | i386/xen: prevent guest from binding loopback event channel to itself | expand |
On 25/07/2023 11:05, David Woodhouse wrote: > From: David Woodhouse <dwmw@amazon.co.uk> > > Fuzzing showed that a guest could bind an interdomain port to itself, by > guessing the next port to be allocated and putting that as the 'remote' > port number. By chance, that works because the newly-allocated port has > type EVTCHNSTAT_unbound. It shouldn't. > > Signed-off-by: David Woodhouse <dwmw@amazon.co.uk> > --- > hw/i386/kvm/xen_evtchn.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > Reviewed-by: Paul Durrant <paul@xen.org>
On Wed, 2023-07-26 at 09:44 +0100, Paul Durrant wrote: > On 25/07/2023 11:05, David Woodhouse wrote: > > From: David Woodhouse <dwmw@amazon.co.uk> > > > > Fuzzing showed that a guest could bind an interdomain port to itself, by > > guessing the next port to be allocated and putting that as the 'remote' > > port number. By chance, that works because the newly-allocated port has > > type EVTCHNSTAT_unbound. It shouldn't. > > > > Signed-off-by: David Woodhouse <dwmw@amazon.co.uk> > > --- > > hw/i386/kvm/xen_evtchn.c | 11 +++++++++-- > > 1 file changed, 9 insertions(+), 2 deletions(-) > > > > Reviewed-by: Paul Durrant <paul@xen.org> > Thanks. I'll change the title prefix to 'hw/xen' since it's in hw/ not target/i386. Please can I have also have a review for https://lore.kernel.org/qemu-devel/20076888f6bdf06a65aafc5cf954260965d45b97.camel@infradead.org/ I'll then send these outstanding patches from my tree as a series for 8.1: David Woodhouse (4): hw/xen: Clarify (lack of) error handling in transaction_commit() hw/xen: fix off-by-one in xen_evtchn_set_gsi() i386/xen: consistent locking around Xen singleshot timers hw/xen: prevent guest from binding loopback event channel to itself
On 26/07/2023 10:07, David Woodhouse wrote: > On Wed, 2023-07-26 at 09:44 +0100, Paul Durrant wrote: >> On 25/07/2023 11:05, David Woodhouse wrote: >>> From: David Woodhouse <dwmw@amazon.co.uk> >>> >>> Fuzzing showed that a guest could bind an interdomain port to itself, by >>> guessing the next port to be allocated and putting that as the 'remote' >>> port number. By chance, that works because the newly-allocated port has >>> type EVTCHNSTAT_unbound. It shouldn't. >>> >>> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk> >>> --- >>> hw/i386/kvm/xen_evtchn.c | 11 +++++++++-- >>> 1 file changed, 9 insertions(+), 2 deletions(-) >>> >> >> Reviewed-by: Paul Durrant <paul@xen.org> >> > > Thanks. I'll change the title prefix to 'hw/xen' since it's in hw/ not > target/i386. Yes, makes sense. > Please can I have also have a review for > https://lore.kernel.org/qemu-devel/20076888f6bdf06a65aafc5cf954260965d45b97.camel@infradead.org/ > Sorry I missed that. Done. Cheers, Paul
Am 26. Juli 2023 09:24:28 UTC schrieb Paul Durrant <xadimgnik@gmail.com>: >On 26/07/2023 10:07, David Woodhouse wrote: >> On Wed, 2023-07-26 at 09:44 +0100, Paul Durrant wrote: >>> On 25/07/2023 11:05, David Woodhouse wrote: >>>> From: David Woodhouse <dwmw@amazon.co.uk> >>>> >>>> Fuzzing showed that a guest could bind an interdomain port to itself, by >>>> guessing the next port to be allocated and putting that as the 'remote' >>>> port number. By chance, that works because the newly-allocated port has >>>> type EVTCHNSTAT_unbound. It shouldn't. >>>> >>>> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk> >>>> --- >>>> hw/i386/kvm/xen_evtchn.c | 11 +++++++++-- >>>> 1 file changed, 9 insertions(+), 2 deletions(-) >>>> >>> >>> Reviewed-by: Paul Durrant <paul@xen.org> >>> >> >> Thanks. I'll change the title prefix to 'hw/xen' since it's in hw/ not >> target/i386. > >Yes, makes sense. > >> Please can I have also have a review for >> https://lore.kernel.org/qemu-devel/20076888f6bdf06a65aafc5cf954260965d45b97.camel@infradead.org/ >> > >Sorry I missed that. Done. And that one, too please? https://lore.kernel.org/qemu-devel/20230720072950.20198-1-olaf@aepfle.de/ Sorry for cross posting, but the patch would be good to have in 8.1. Best regards, Bernhard > >Cheers, > > Paul >
diff --git a/hw/i386/kvm/xen_evtchn.c b/hw/i386/kvm/xen_evtchn.c index 0e9c108614..70b4b8a6ef 100644 --- a/hw/i386/kvm/xen_evtchn.c +++ b/hw/i386/kvm/xen_evtchn.c @@ -1408,8 +1408,15 @@ int xen_evtchn_bind_interdomain_op(struct evtchn_bind_interdomain *interdomain) XenEvtchnPort *rp = &s->port_table[interdomain->remote_port]; XenEvtchnPort *lp = &s->port_table[interdomain->local_port]; - if (rp->type == EVTCHNSTAT_unbound && rp->type_val == 0) { - /* It's a match! */ + /* + * The 'remote' port for loopback must be an unbound port allocated for + * communication with the local domain (as indicated by rp->type_val + * being zero, not PORT_INFO_TYPEVAL_REMOTE_QEMU), and must *not* be + * the port that was just allocated for the local end. + */ + if (interdomain->local_port != interdomain->remote_port && + rp->type == EVTCHNSTAT_unbound && rp->type_val == 0) { + rp->type = EVTCHNSTAT_interdomain; rp->type_val = interdomain->local_port;