On 11/07/2022 17:08, Rahul Singh wrote:
Hi Julien,
Hi Rahul,
On 22 Jun 2022, at 3:51 pm, Julien Grall <jul...@xen.org> wrote:
Hi,
On 22/06/2022 15:37, Rahul Singh wrote:
evtchn_alloc_unbound() always allocates the next available port. Static
event channel support for dom0less domains requires allocating a
specified port.
Modify the evtchn_alloc_unbound() to accept the port number as an
argument and allocate the specified port if available. If the port
number argument is zero, the next available port will be allocated.
I haven't yet fully reviewed this series. But I would like to point out that
this opening a security hole (which I thought I had mention before) that could
be exploited by a guest at runtime.
You would need [1] or similar in order to fix the issue. I am wrote "similar"
because the patch could potentially be a problem if you allow a guest to use FIFO (you
may need to allocate a lot of memory to fill the hole).
Cheers,
[1]
https://xenbits.xen.org/gitweb/?p=people/julieng/xen-unstable.git;a=commit;h=2d89486fcf11216331e58a21b367b8a9be1af725
Thanks for sharing the patch. If you are okay I can use this patch in next
version to fix the security hole.
I am fine with that.
For the FIFO issue, we can introduce the new config option to restrict the
maximum number of static
port supported in Xen. We can check the user-defined static port when we parse
the device tree and if
a user-defined static port is greater than the maximum allowed static port will
return an error to the user.
In this way, we can avoid allocating a lot of memory to fill the hole.
Let me know your view on this.
config MAX_STATIC_PORT
int "Maximum number of static ports”
range 1 4095
help
Controls the build-time maximum number of static port supported
The problem is not exclusive to the static event channel. So I don't
think this is right to introduce MAX_STATIC_PORT to mitigate the issue
(even though this is the only user today).
A few of alternative solutions:
1) Handle preemption in alloc_evtchn_bucket()
2) Allocate all the buckets when the domain is created (the max
numbers event channel is known). We may need to think about preemption
3) Tweak is_port_valid() to check if the bucket is valid. This would
introduce a couple of extra memory access (might be OK as the bucket
would be accessed afterwards) and we would need to update some users.
At the moment, 3) is appealing me the most. I would be interested to
have an opionions from the other maintainers.
Cheers,
--
Julien Grall
