File 5e3bd385-EFI-recheck-variable-name-strings.patch of Package xen.14764
# Commit ad38db5852f0e30d90c93c6a62b754f2861549e0
# Date 2020-02-06 09:51:17 +0100
# Author Jan Beulich <jbeulich@suse.com>
# Committer Jan Beulich <jbeulich@suse.com>
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@ioactive.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: George Dunlap <george.dunlap@citrix.com>
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -283,16 +283,6 @@ static int __init wstrncmp(const CHAR16
return n ? *s1 - *s2 : 0;
}
-static const CHAR16 *__init wmemchr(const CHAR16 *s, CHAR16 c, UINTN n)
-{
- while ( n && *s != c )
- {
- --n;
- ++s;
- }
- return n ? s : NULL;
-}
-
static CHAR16 *__init s2w(union string *str)
{
const char *s = str->s;
--- a/xen/common/efi/efi.h
+++ b/xen/common/efi/efi.h
@@ -39,3 +39,5 @@ extern UINT64 efi_boot_max_var_store_siz
extern UINT64 efi_apple_properties_addr;
extern UINTN efi_apple_properties_len;
+
+const CHAR16 *wmemchr(const CHAR16 *s, CHAR16 c, UINTN n);
--- a/xen/common/efi/runtime.c
+++ b/xen/common/efi/runtime.c
@@ -194,7 +194,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 +479,12 @@ int efi_runtime_call(struct xenpf_efi_ru
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 +532,12 @@ int efi_runtime_call(struct xenpf_efi_ru
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 )