[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-changelog] [xen staging] EFI: re-check {get, set}-variable name strings after copying in
commit ad38db5852f0e30d90c93c6a62b754f2861549e0 Author: Jan Beulich <jbeulich@xxxxxxxx> AuthorDate: Thu Feb 6 09:51:17 2020 +0100 Commit: Jan Beulich <jbeulich@xxxxxxxx> CommitDate: Thu Feb 6 09:51:17 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> --- xen/common/efi/boot.c | 10 ---------- xen/common/efi/efi.h | 2 ++ xen/common/efi/runtime.c | 27 ++++++++++++++++++++++++--- 3 files changed, 26 insertions(+), 13 deletions(-) diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c index b9f461505c..a6f84c945a 100644 --- a/xen/common/efi/boot.c +++ b/xen/common/efi/boot.c @@ -281,16 +281,6 @@ static int __init wstrncmp(const CHAR16 *s1, const CHAR16 *s2, UINTN n) 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; diff --git a/xen/common/efi/efi.h b/xen/common/efi/efi.h index 6b9c56ead1..2e38d05f3d 100644 --- a/xen/common/efi/efi.h +++ b/xen/common/efi/efi.h @@ -39,3 +39,5 @@ extern UINT64 efi_boot_max_var_store_size, efi_boot_remain_var_store_size, extern UINT64 efi_apple_properties_addr; extern UINTN efi_apple_properties_len; + +const CHAR16 *wmemchr(const CHAR16 *s, CHAR16 c, UINTN n); diff --git a/xen/common/efi/runtime.c b/xen/common/efi/runtime.c index 8c2ece468d..752e604390 100644 --- 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) @@ -465,7 +476,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 ) @@ -513,7 +529,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#staging _______________________________________________ Xen-changelog mailing list Xen-changelog@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/xen-changelog
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |