[PATCH v3] evtchn/fifo: don't enforce higher than necessary alignment

[Jan Beulich <jbeulich@xxxxxxxx>]

Neither the code nor the original commit provide any justification for
the need to 8-byte align the struct in all cases. Enforce just as much
alignment as the structure actually needs - 4 bytes - by using alignof()
instead of a literal number.

While relaxation of the requirements is intended here, the primary goal
is to simply get rid of the hard coded number as well its lack of
connection to the structure that is is meant to apply to.

Take the opportunity and also
- add so far missing validation that native and compat mode layouts of
  the structures actually match,
- tie sizeof() expressions to the types of the fields we're actually
  after, rather than specifying the type explicitly (which in the
  general case risks a disconnect, even if there's close to zero risk in
  this particular case),
- use ENXIO instead of EINVAL for the two cases of the address not
  satisfying the requirements, which will make an issue here better
  stand out at the call site.

Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
---
v3: Adjust version in public header comment.
v2: Add comment to public header. Re-base.
---
I question the need for the array_index_nospec() here. Or else I'd
expect map_vcpu_info() would also need the same.

--- a/xen/common/event_fifo.c
+++ b/xen/common/event_fifo.c
@@ -567,6 +567,16 @@ static void setup_ports(struct domain *d
     }
 }
 
+#ifdef CONFIG_COMPAT
+
+#include <compat/event_channel.h>
+
+#define xen_evtchn_fifo_control_block evtchn_fifo_control_block
+CHECK_evtchn_fifo_control_block;
+#undef xen_evtchn_fifo_control_block
+
+#endif
+
 int evtchn_fifo_init_control(struct evtchn_init_control *init_control)
 {
     struct domain *d = current->domain;
@@ -586,19 +596,20 @@ int evtchn_fifo_init_control(struct evtc
         return -ENOENT;
 
     /* Must not cross page boundary. */
-    if ( offset > (PAGE_SIZE - sizeof(evtchn_fifo_control_block_t)) )
-        return -EINVAL;
+    if ( offset > (PAGE_SIZE - sizeof(*v->evtchn_fifo->control_block)) )
+        return -ENXIO;
 
     /*
      * Make sure the guest controlled value offset is bounded even during
      * speculative execution.
      */
     offset = array_index_nospec(offset,
-                           PAGE_SIZE - sizeof(evtchn_fifo_control_block_t) + 
1);
+                                PAGE_SIZE -
+                                sizeof(*v->evtchn_fifo->control_block) + 1);
 
-    /* Must be 8-bytes aligned. */
-    if ( offset & (8 - 1) )
-        return -EINVAL;
+    /* Must be suitably aligned. */
+    if ( offset & (alignof(*v->evtchn_fifo->control_block) - 1) )
+        return -ENXIO;
 
     spin_lock(&d->event_lock);
 
--- a/xen/include/public/event_channel.h
+++ b/xen/include/public/event_channel.h
@@ -368,6 +368,11 @@ typedef uint32_t event_word_t;
 
 #define EVTCHN_FIFO_NR_CHANNELS (1 << EVTCHN_FIFO_LINK_BITS)
 
+/*
+ * While this structure only requires 4-byte alignment, Xen versions 4.15 and
+ * earlier reject offset values (in struct evtchn_init_control) that aren't a
+ * multiple of 8.
+ */
 struct evtchn_fifo_control_block {
     uint32_t ready;
     uint32_t _rsvd;
--- a/xen/include/xlat.lst
+++ b/xen/include/xlat.lst
@@ -67,6 +67,7 @@
 ?      evtchn_bind_virq                event_channel.h
 ?      evtchn_close                    event_channel.h
 ?      evtchn_expand_array             event_channel.h
+?      evtchn_fifo_control_block       event_channel.h
 ?      evtchn_init_control             event_channel.h
 ?      evtchn_op                       event_channel.h
 ?      evtchn_reset                    event_channel.h