|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [xen master] lib: make safe_copy_string_from_guest() validate input
commit 508209c47305f6655b36933e1bd96a682a438aaa
Author: Jan Beulich <jbeulich@xxxxxxxx>
AuthorDate: Mon Jul 6 09:42:10 2026 +0200
Commit: Jan Beulich <jbeulich@xxxxxxxx>
CommitDate: Mon Jul 6 09:42:10 2026 +0200
lib: make safe_copy_string_from_guest() validate input
... rather than papering over guest flaws: Strings passed ought to be nul-
terminated (yet sadly libxc hasn't been doing so thus far). This way we
also avoid order-1 allocations, seeing that all present callers pass
PAGE_SIZE for max_size.
Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
Reviewed-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>
Acked-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx> # Changelog
---
CHANGELOG.md | 2 ++
xen/include/public/domctl.h | 2 +-
xen/include/public/xsm/flask_op.h | 12 ++++++++----
xen/lib/guest-strcpy.c | 14 +++++++++-----
4 files changed, 20 insertions(+), 10 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 0a4e5f013c..d8287fc311 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,6 +7,8 @@ The format is based on [Keep a
Changelog](https://keepachangelog.com/en/1.0.0/)
## [4.23.0
UNRELEASED](https://xenbits.xenproject.org/gitweb/?p=xen.git;a=shortlog;h=staging)
- TBD
### Changed
+ - XEN_DOMCTL_DEV_DT's, FLASK_[GS]ETBOOL's, and FLASK_DEVICETREE_LABEL's input
+ string sizes need to include the nul terminator.
### Added
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index cdf350a290..510300bb67 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -574,7 +574,7 @@ struct xen_domctl_assign_device {
uint32_t machine_sbdf; /* machine PCI ID of assigned device */
} pci;
struct {
- uint32_t size; /* Length of the path */
+ uint32_t size; /* Length of the path, including nul terminator */
XEN_GUEST_HANDLE_64(char) path; /* Path to the device tree node */
#ifdef __XEN__
struct dt_device_node *dev; /* Resolved device node of the above */
diff --git a/xen/include/public/xsm/flask_op.h
b/xen/include/public/xsm/flask_op.h
index 7185e80621..10e4ffe86e 100644
--- a/xen/include/public/xsm/flask_op.h
+++ b/xen/include/public/xsm/flask_op.h
@@ -26,7 +26,8 @@ typedef struct xen_flask_setenforce xen_flask_setenforce_t;
struct xen_flask_sid_context {
/* IN/OUT: sid to convert to/from string */
uint32_t sid;
- /* IN: size of the context buffer
+ /*
+ * IN: size of the context buffer, including nul terminator
* OUT: actual size of the output context string
*/
uint32_t size;
@@ -86,8 +87,11 @@ struct xen_flask_boolean {
uint8_t new_value;
/* IN: commit new value instead of only setting pending [SET] */
uint8_t commit;
- /* IN: size of boolean name buffer [GET/SET]
- * OUT: actual size of name [GET only] */
+ /*
+ * IN: size of boolean name buffer [GET/SET]; must cover nul terminator
+ * if "name" (below) is an input
+ * OUT: actual size of name [GET only]
+ */
uint32_t size;
/* IN: if bool_id is -1, used to find boolean [GET/SET]
* OUT: textual name of boolean [GET only]
@@ -150,7 +154,7 @@ typedef struct xen_flask_relabel xen_flask_relabel_t;
struct xen_flask_devicetree_label {
/* IN */
uint32_t sid;
- uint32_t length;
+ uint32_t length; /* length of the path, including nul terminator */
XEN_GUEST_HANDLE(char) path;
};
typedef struct xen_flask_devicetree_label xen_flask_devicetree_label_t;
diff --git a/xen/lib/guest-strcpy.c b/xen/lib/guest-strcpy.c
index 6d38eefedd..94e7e38eda 100644
--- a/xen/lib/guest-strcpy.c
+++ b/xen/lib/guest-strcpy.c
@@ -3,8 +3,8 @@
#include <xen/err.h>
/*
- * The function copies a string from the guest and adds a NUL to
- * make sure the string is correctly terminated.
+ * The function copies a string from the guest and checks there's a NUL
+ * terminating the string.
*/
char *safe_copy_string_from_guest(XEN_GUEST_HANDLE(char) u_buf,
size_t size, size_t max_size)
@@ -14,8 +14,7 @@ char *safe_copy_string_from_guest(XEN_GUEST_HANDLE(char)
u_buf,
if ( size > max_size )
return ERR_PTR(-ENOBUFS);
- /* Add an extra +1 to append \0 */
- tmp = xmalloc_array(char, size + 1);
+ tmp = xmalloc_array(char, size);
if ( !tmp )
return ERR_PTR(-ENOMEM);
@@ -24,7 +23,12 @@ char *safe_copy_string_from_guest(XEN_GUEST_HANDLE(char)
u_buf,
xfree(tmp);
return ERR_PTR(-EFAULT);
}
- tmp[size] = '\0';
+
+ if ( !memchr(tmp, 0, size) )
+ {
+ xfree(tmp);
+ return ERR_PTR(-EMSGSIZE);
+ }
return tmp;
}
--
generated by git-patchbot for /home/xen/git/xen.git#master
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |