Guard every mutation of serial_rx_cons/prod with console_lock so that
cross-domain reads can't see stale data:

- In console_switch_input(): protect console_rx assignment with the lock
  using irqsave/irqrestore variants since this can be called from
  interrupt context

- In __serial_rx(): protect the ring buffer write operation when
  delivering input to the hardware domain

- In do_console_io() CONSOLEIO_read: hold the lock around the entire
  read loop, using a local buffer copy to avoid holding the lock during
  copy_to_guest_offset()

This is preparatory work for allowing multiple domains to use the
console_io hypercalls where proper synchronization is required.

Signed-off-by: Stefano Stabellini <[email protected]>
---
Changes in v10:
- patch split into two halves, this is the first half
---
 xen/drivers/char/console.c | 26 +++++++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index fbc89ca2a4..ad53073b99 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -553,10 +553,13 @@ static void console_switch_input(void)
     {
         domid_t domid;
         struct domain *d;
+        unsigned long flags;
 
         if ( next_rx++ >= max_console_rx )
         {
+            nrspin_lock_irqsave(&console_lock, flags);
             ACCESS_ONCE(console_rx) = 0;
+            nrspin_unlock_irqrestore(&console_lock, flags);
             printk("*** Serial input to Xen");
             break;
         }
@@ -576,7 +579,9 @@ static void console_switch_input(void)
 
             rcu_unlock_domain(d);
 
+            nrspin_lock_irqsave(&console_lock, flags);
             ACCESS_ONCE(console_rx) = next_rx;
+            nrspin_unlock_irqrestore(&console_lock, flags);
             printk("*** Serial input to DOM%u", domid);
             break;
         }
@@ -602,12 +607,16 @@ static void __serial_rx(char c)
 
     if ( is_hardware_domain(d) )
     {
+        unsigned long flags;
+
         /*
          * Deliver input to the hardware domain buffer, unless it is
          * already full.
          */
+        nrspin_lock_irqsave(&console_lock, flags);
         if ( (serial_rx_prod - serial_rx_cons) != SERIAL_RX_SIZE )
             serial_rx_ring[SERIAL_RX_MASK(serial_rx_prod++)] = c;
+        nrspin_unlock_irqrestore(&console_lock, flags);
 
         /*
          * Always notify the hardware domain: prevents receive path from
@@ -796,6 +805,7 @@ long do_console_io(
 {
     long rc;
     unsigned int idx, len;
+    char kbuf[SERIAL_RX_SIZE];
 
     rc = xsm_console_io(XSM_OTHER, current->domain, cmd);
     if ( rc )
@@ -817,6 +827,7 @@ long do_console_io(
             break;
 
         rc = 0;
+        nrspin_lock_irq(&console_lock);
         while ( (serial_rx_cons != serial_rx_prod) && (rc < count) )
         {
             idx = SERIAL_RX_MASK(serial_rx_cons);
@@ -825,14 +836,19 @@ long do_console_io(
                 len = SERIAL_RX_SIZE - idx;
             if ( (rc + len) > count )
                 len = count - rc;
-            if ( copy_to_guest_offset(buffer, rc, &serial_rx_ring[idx], len) )
-            {
-                rc = -EFAULT;
-                break;
-            }
+            memcpy(kbuf, &serial_rx_ring[idx], len);
+
+            nrspin_unlock_irq(&console_lock);
+
+            if ( copy_to_guest_offset(buffer, rc, kbuf, len) )
+               return -EFAULT;
             rc += len;
+
+            nrspin_lock_irq(&console_lock);
+
             serial_rx_cons += len;
         }
+        nrspin_unlock_irq(&console_lock);
         break;
     default:
         rc = -ENOSYS;
-- 
2.25.1


Reply via email to