[Xen-changelog] [xen stable-4.11] EFI: re-check {get, set}-variable name strings after copying in

[patchbot@xxxxxxx]

commit a4f502e03023253a14fe1ed06ab7011a0f08e647
Author:     Jan Beulich <jbeulich@xxxxxxxx>
AuthorDate: Thu Mar 5 11:33:26 2020 +0100
Commit:     Jan Beulich <jbeulich@xxxxxxxx>
CommitDate: Thu Mar 5 11:33:26 2020 +0100

    EFI: re-check {get,set}-variable name strings after copying in
    
    A malicious guest given permission to invoke XENPF_efi_runtime_call may
    play with the strings underneath Xen sizing them and copying them in.
    Guard against this by re-checking the copyied in data for consistency
    with the initial sizing. At the same time also check that the actual
    copy-in is in fact successful, and switch to the lighter weight non-
    checking flavor of the function.
    
    Reported-by: Ilja Van Sprundel <ivansprundel@xxxxxxxxxxxx>
    Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
    Reviewed-by: George Dunlap <george.dunlap@xxxxxxxxxx>
    master commit: ad38db5852f0e30d90c93c6a62b754f2861549e0
    master date: 2020-02-06 09:51:17 +0100
---
 xen/common/efi/runtime.c | 29 ++++++++++++++++++++++++++---
 1 file changed, 26 insertions(+), 3 deletions(-)

diff --git a/xen/common/efi/runtime.c b/xen/common/efi/runtime.c
index 3d118d571d..2427d22ab3 100644
--- a/xen/common/efi/runtime.c
+++ b/xen/common/efi/runtime.c
@@ -27,6 +27,8 @@ struct efi_rs_state {
 struct efi_rs_state efi_rs_enter(void);
 void efi_rs_leave(struct efi_rs_state *);
 
+const CHAR16 *wmemchr(const CHAR16 *s, CHAR16 c, UINTN n);
+
 #ifndef COMPAT
 
 #ifndef CONFIG_ARM
@@ -194,7 +196,18 @@ void efi_reset_system(bool warm)
 }
 
 #endif /* CONFIG_ARM */
-#endif
+
+const CHAR16 *wmemchr(const CHAR16 *s, CHAR16 c, UINTN n)
+{
+    while ( n && *s != c )
+    {
+        --n;
+        ++s;
+    }
+    return n ? s : NULL;
+}
+
+#endif /* COMPAT */
 
 #ifndef CONFIG_ARM /* TODO - disabled until implemented on ARM */
 int efi_get_info(uint32_t idx, union xenpf_efi_info *info)
@@ -468,7 +481,12 @@ int efi_runtime_call(struct xenpf_efi_runtime_call *op)
         name = xmalloc_array(CHAR16, ++len);
         if ( !name )
            return -ENOMEM;
-        __copy_from_guest(name, op->u.get_variable.name, len);
+        if ( __copy_from_guest(name, op->u.get_variable.name, len) ||
+             wmemchr(name, 0, len) != name + len - 1 )
+        {
+            xfree(name);
+            return -EIO;
+        }
 
         size = op->u.get_variable.size;
         if ( size )
@@ -516,7 +534,12 @@ int efi_runtime_call(struct xenpf_efi_runtime_call *op)
         name = xmalloc_array(CHAR16, ++len);
         if ( !name )
            return -ENOMEM;
-        __copy_from_guest(name, op->u.set_variable.name, len);
+        if ( __copy_from_guest(name, op->u.set_variable.name, len) ||
+             wmemchr(name, 0, len) != name + len - 1 )
+        {
+            xfree(name);
+            return -EIO;
+        }
 
         data = xmalloc_bytes(op->u.set_variable.size);
         if ( !data )
--
generated by git-patchbot for /home/xen/git/xen.git#stable-4.11

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/xen-changelog