Hi Michal, > On 1 Sep 2022, at 1:47 pm, Michal Orzel <michal.or...@amd.com> wrote: > > Hi Rahul, > > On 01/09/2022 11:13, Rahul Singh wrote: >> >> From: Julien Grall <jgr...@amazon.com> >> >> Since commit 01280dc19cf3 "evtchn: simplify port_is_valid()", the event >> channels code assumes that all the buckets below d->valid_evtchns are >> always allocated. >> >> This assumption hold in most of the situation because a guest is not >> allowed to chose the port. Instead, it will be the first free from port >> 0. >> >> When static event channel support will be added for dom0less domains >> user can request to allocate the evtchn port numbers that are scattered >> in nature. >> >> The existing implementation of evtchn_allocate_port() is not able to >> deal with such situation and will end up to override bucket or/and leave >> some bucket unallocated. The latter will result to a droplet crash if >> the event channel belongs to an unallocated bucket. >> >> This can be solved by making sure that all the buckets below >> d->valid_evtchns are allocated. There should be no impact for most of >> the situation but LM/LU as only one bucket would be allocated. For >> LM/LU, we may end up to allocate multiple buckets if ports in use are >> sparse. >> >> A potential alternative is to check that the bucket is valid in >> is_port_valid(). This should still possible to do it without taking >> per-domain lock but will result a couple more of memory access. >> >> Signed-off-by: Julien Grall <jgr...@amazon.com> >> Signed-off-by: Rahul Singh <rahul.si...@arm.com> >> --- >> Changes in v3: >> - fix comments in commit msg. >> - modify code related to d->valid_evtchns and {read,write}_atomic() >> Changes in v2: >> - new patch in this version to avoid the security issue >> --- >> xen/common/event_channel.c | 55 ++++++++++++++++++++++++-------------- >> 1 file changed, 35 insertions(+), 20 deletions(-) >> >> diff --git a/xen/common/event_channel.c b/xen/common/event_channel.c >> index c2c6f8c151..80b06d9743 100644 >> --- a/xen/common/event_channel.c >> +++ b/xen/common/event_channel.c >> @@ -193,6 +193,15 @@ static struct evtchn *alloc_evtchn_bucket(struct domain >> *d, unsigned int port) >> return NULL; >> } >> >> +/* >> + * Allocate a given port and ensure all the buckets up to that ports >> + * have been allocated. >> + * >> + * The last part is important because the rest of the event channel code >> + * relies on all the buckets up to d->valid_evtchns to be valid. However, >> + * event channels may be sparsed when restoring a domain during Guest >> + * Transparent Migration and Live Update. > You got rid of mentioning these non-existing features from the commit msg, > but you still mention them here.
I missed that I will fix that in next version. Regards, Rahul