fixup! [LibOS] Emulate in/out instructions as if they generate SIGSEGV

Signed-off-by: Nirjhar Roy <nirjhar.roy@fortanix.com>

common/include/arch/x86_64/cpu.h

@@ -2,6 +2,7 @@
 
 #pragma once
 
+#include <stdbool.h>
 #include <stdint.h>
 #include <stdnoreturn.h>
 
@@ -51,6 +52,8 @@ enum extended_state_sub_leaf {
 #define CPU_BRAND_CNTD2_LEAF             0x80000004
 #define INVARIANT_TSC_LEAF               0x80000007
 
+bool is_x86_instr_legacy_prefix(uint8_t op);
+
 static inline void cpuid(unsigned int leaf, unsigned int subleaf, unsigned int words[static 4]) {
     __asm__("cpuid"
             : "=a"(words[CPUID_WORD_EAX]),

common/src/arch/x86_64/cpu.c

@@ -0,0 +1,43 @@
+/* SPDX-License-Identifier: LGPL-3.0-or-later */
+/* Copyright (C) 2022 Fortnanix Inc
+ *                    Nirjhar Roy <nirjhar.roy@fortanix.com>
+ */
+
+/* This file contains functions that check various features and flags
+ * specific to x86
+ */
+
+#include <stddef.h>
+
+#include "api.h"
+#include "cpu.h"
+
+bool is_x86_instr_legacy_prefix(uint8_t op) {
+    uint8_t prefix_list[] = {
+                             /* Group 0 */
+                             0xf0, /* LOCK prefix */
+                             0xf2, /* REPNE/REPNZ prefix */
+                             0xf3, /* REP or REPE/REPZ prefix */
+                             /* Group 1 */
+                             0x2e, /* CS segment override */
+                             0x36, /* SS segment override */
+                             0x3e, /* DS segment override */
+                             0x26, /* ES segment override */
+                             0x64, /* FS segment override */
+                             0x65, /* GS segment override */
+                             0x2e, /* Branch not taken */
+                             0x3e, /* Branch taken */
+                             /* Group 2 */
+                             0x66, /*  Operand-size override prefix */
+                             /* Group 3 */
+                             0x67, /* Address-size override prefix  */
+                             /* The rest of the prefixes aren't really applicable
+                              * for the instruction(s) we are checking.
+                              */
+                            };
+    for (size_t i = 0; i < ARRAY_SIZE(prefix_list); i++) {
+        if (op == prefix_list[i])
+            return true;
+    }
+    return false;
+}

common/src/arch/x86_64/meson.build

@@ -2,7 +2,7 @@ common_src_arch_nasm = nasm_gen.process(
     'ct_memequal.nasm',
 )
 
-common_src_arch_c = files()
+common_src_arch_c = files('cpu.c')
 
 common_src_arch = [
     common_src_arch_nasm,

libos/src/bookkeep/libos_signal.c

@@ -459,82 +459,37 @@ bool is_user_string_readable(const char* addr) {
     }
 }
 
-static bool is_legacy_prefix(uint8_t op) {
-    uint8_t prefix_list[] = {
-                             /* Group 0 */ 
-                             0xf0, /* LOCK prefix */
-                             0xf2, /* REPNE/REPNZ prefix */
-                             0xf3, /* REP or REPE/REPZ prefix */
-                             /* Group 1 */
-                             0x2e, /* CS segment override */
-                             0x36, /* SS segment override */
-                             0x3e, /* DS segment override */
-                             0x26, /* ES segment override */
-                             0x64, /* FS segment override */
-                             0x65, /* GS segment override */
-                             0x2e, /* Branch not taken */
-                             0x3e, /* Branch taken */
-                             /* Group 2 */ 
-                             0x66, /*  Operand-size override prefix */
-                             /* Group 3 */
-                             0x67, /* Address-size override prefix  */
-                             /* The rest of the prefixes aren't really applicable 
-                              * for the instruction(s) we are checking.
-                              */
-                            };
-    size_t num_prefixes = ARRAY_SIZE(prefix_list);
-    for (size_t i = 0; i < num_prefixes; i++) {
-        if (op == prefix_list[i])
-            return true;
-    }
-    return false;
-}
-
 static bool is_in_out(PAL_CONTEXT* context) {
     uint8_t opcodes[] = {
-                        /* INS opcodes */
-                        0x6c,
-                        0x6d,
-                        /* OUTS opcodes */
-                        0x6e,
-                        0x6f,
-                        /* IN immediate opcodes */
-                        0xe4,
-                        0xe5,
-                        /* OUT immediate opcodes */
-                        0xe6,
-                        0xe7,
-                        /* IN register opcodes */
-                        0xec,
-                        0xed,
-                        /* OUT register opcodes */
-                        0xee,
-                        0xef,                        
+                        0x6c, 0x6d, /* INS opcodes */
+                        0x6e, 0x6f, /* OUTS opcodes */
+                        0xe4, 0xe5, /* IN immediate opcodes */
+                        0xe6, 0xe7, /* OUT immediate opcodes */
+                        0xec, 0xed, /* IN register opcodes */
+                        0xee, 0xef, /* OUT register opcodes */
                         };
-    size_t num_opcodes = ARRAY_SIZE(opcodes);
     uint8_t* rip = (uint8_t*)context->rip;
-    size_t num_prefixes_to_check = 4;
-    /* num_prefixes_found will store the actual opcode index in rip */
-    size_t num_prefixes_found = 0;
-    for (size_t i = 0; i < num_prefixes_to_check; i++) {
-        bool is_prefix = is_legacy_prefix(rip[i]);
-        num_prefixes_found += (is_prefix ? 1 : 0);
-        if (!is_prefix) 
-            break;
-    }
-    for (size_t i = 0; i < num_opcodes; i++) {
-        if (rip[num_prefixes_found] == opcodes[i]) {
+    size_t idx = 0;
+    while (is_x86_instr_legacy_prefix(rip[idx]) && idx < 4)
+        idx++;
+    for (size_t i = 0; i < ARRAY_SIZE(opcodes); i++)
+        if (rip[idx] == opcodes[i])
             return true;
-        }
-    }
     return false;
 }
 
 static bool maybe_raise_sigsegv(PAL_CONTEXT* context) {
+    /* Executing I/O instructions (e.g., in/out) inside an SGX enclave
+     * generates a #UD fault. Gramine's PAL tries to handle this exception and
+     * propogates it to LibOS/app as a SIGILL signal.
+     *
+     * However, I/O instructions result in a #GP fault (which raises a SIGSEGV
+     * signal) if I/O is not permitted. Let Gramine emulate these instructions
+     * as if they end up in SIGSEGV. This helps some apps, e.g. `lscpu`.
+     */
     return is_in_out(context);
 }
 
-
 static void illegal_upcall(bool is_in_pal, uintptr_t addr, PAL_CONTEXT* context) {
     __UNUSED(is_in_pal);
     assert(!is_in_pal);
@@ -559,18 +514,10 @@ static void illegal_upcall(bool is_in_pal, uintptr_t addr, PAL_CONTEXT* context)
             .si_addr = (void*)addr,
         };
         if (maybe_raise_sigsegv(context)) {
-            /* Executing I/O instructions (e.g., in/out) inside an SGX enclave 
-             * generates a #UD fault. Gramine's PAL tries to handle this exception and
-             *  propogates it to LibOS/app as a SIGILL signal.
-             * 
-             * However, I/O instructions result in a #GP fault (which raises a SIGSEGV
-             * signal) if I/O is not permitted. Let Gramine emulate these instructions as
-             * if they end up in SIGSEGV. This helps some apps, e.g. `lscpu`.
-             */ 
             info.si_signo = SIGSEGV;
             info.si_code = SEGV_MAPERR;
-            log_debug("Illegal instruction during app execution at %p, emulated as if throwing SIGSEGV;"
-                      " delivering to app", rip);
+            log_debug("Illegal instruction during app execution at %p, emulated as if "
+                      "throwing SIGSEGV; delivering to app", rip);
         } else {
             log_debug("Illegal instruction during app execution at %p; delivering to app", rip);
         }

libos/test/regression/in_out_instruction_test.c

@@ -1,5 +1,5 @@
-/* Test Description: This test verifies that in and out instructions 
- * correctly generate SIGSEGV. This raises SIGSEGV once for in and once for out  
+/* Test Description: This test verifies that in and out instructions
+ * correctly generate SIGSEGV. This raises SIGSEGV once for IN and once for OUT
  * and then counts if number of SIGSEGVs are 2.
  */
 #define _XOPEN_SOURCE 700
@@ -10,36 +10,37 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <signal.h>
+
 #include "common.h"
 
 #define EXPECTED_NUM_SIGSEGVS 2
 static int g_sigsegv_triggered = 0;
-static jmp_buf g_point;
+static sigjmp_buf g_point;
 
 static void fault_handler(int signum) {
     assert(signum == SIGSEGV);
     g_sigsegv_triggered++;
     siglongjmp(g_point, 1);
-    return;
 }
 
 int main(void) {
     struct sigaction int_handler = {.sa_handler=fault_handler,
                                     .sa_flags = SA_RESTART};
+    int value = 0;
+    int port = 0;
     sigaction(SIGSEGV, &int_handler, 0);
-    if (CHECK(sigsetjmp(g_point, 1)) == 0) {
-        __asm__("mov $0, %al;");
-        __asm__("mov $0, %dx;");
-        __asm__("in %dx, %al;");/* AT & T style */
+    if (sigsetjmp(g_point, 1) == 0) {
+        __asm__ volatile("in %1, %0" : "=a"(value) : "d"(port));
     }
-    puts("handled in instruction\n");
-    if (CHECK(sigsetjmp(g_point, 1)) == 0) {
-        __asm__("mov $0, %al;");
-        __asm__("mov $0, %dx;");
-        __asm__("out %al, %dx;");/* AT & T style */
+    puts("handled IN instruction");
+    if (sigsetjmp(g_point, 1) == 0) {
+        port = 0;
+        __asm__ volatile("out %0, %1" : "=a"(value) : "d"(port));
     }
-    puts("handled out instruction\n");
-    assert(g_sigsegv_triggered == EXPECTED_NUM_SIGSEGVS);
-    puts("SIGSEGV TEST OK");
+    puts("handled OUT instruction");
+    if (g_sigsegv_triggered == EXPECTED_NUM_SIGSEGVS)
+        puts("SIGSEGV TEST OK");
+    else
+        puts("SIGSEGV TEST FAILED");
     return 0;
-}
+}

libos/test/regression/test_libos.py

@@ -73,10 +73,6 @@ def test_001_helloworld(self):
     def test_002_toml_parsing(self):
         stdout, _ = self.run_binary(['toml_parsing'])
         self.assertIn('Hello world!', stdout)
-
-    def test_003_copy_in_out_ins_test(self):
-        stdout, stderr = self.run_binary(['in_out_instruction_test'])
-        self.assertIn('SIGSEGV TEST OK', stdout)
 
     def test_100_basic_bootstrapping(self):
         stdout, _ = self.run_binary(['bootstrap'])
@@ -1342,3 +1338,8 @@ class TC_92_avx(RegressionTestCase):
     def test_000_avx(self):
         stdout, _ = self.run_binary(['avx'])
         self.assertIn('TEST OK', stdout)
+
+class TC_93_In_Out(RegressionTestCase):
+    def test_000_in_out(self):
+        stdout, stderr = self.run_binary(['in_out_instruction_test'])
+        self.assertIn('SIGSEGV TEST OK', stdout)