|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 14/22] libelf: use C99 bool for booleans
On 07/06/13 19:27, Ian Jackson wrote:
> We want to remove uses of "int" because signed integers have
> undesirable undefined behaviours on overflow. Malicious compilers can
> turn apparently-correct code into code with security vulnerabilities
> etc.
>
> In this patch we change all the booleans in libelf to C99 bool,
> from <stdbool.h>.
>
> For the one visible libelf boolean in libxc's public interface we
> retain the use of int to avoid changing the ABI; libxc converts it to
> a bool for consumption by libelf.
>
> It is OK to change all values only ever used as booleans to _Bool
> (bool) because conversion from any scalar type to a _Bool works the
> same as the boolean test in if() or ?: and is always defined (C99
> 6.3.1.2). But we do need to check that all these variables really are
> only ever used that way. (It is theoretically possible that the old
> code truncated some 64-bit values to 32-bit ints which might become
> zero depending on the value, which would mean a behavioural change in
> this patch, but it seems implausible that treating 0x????????00000000
> as false could have been intended.)
>
> This is part of the fix to a security issue, XSA-55.
>
> Signed-off-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> Acked-by: George Dunlap <george.dunlap@xxxxxxxxxxxxx>
>
> v3: Use <stdbool.h>'s bool (or _Bool) instead of defining elf_bool.
> Split this into a separate patch.
> ---
> tools/libxc/xc_dom_elfloader.c | 8 ++++----
> xen/common/libelf/libelf-dominfo.c | 2 +-
> xen/common/libelf/libelf-loader.c | 4 ++--
> xen/common/libelf/libelf-private.h | 2 +-
> xen/common/libelf/libelf-tools.c | 10 +++++-----
> xen/include/xen/libelf.h | 18 ++++++++++--------
> 6 files changed, 23 insertions(+), 21 deletions(-)
>
> diff --git a/tools/libxc/xc_dom_elfloader.c b/tools/libxc/xc_dom_elfloader.c
> index a0d39b3..8f9c2fb 100644
> --- a/tools/libxc/xc_dom_elfloader.c
> +++ b/tools/libxc/xc_dom_elfloader.c
> @@ -34,7 +34,7 @@
> /* ------------------------------------------------------------------------
> */
>
> static void log_callback(struct elf_binary *elf, void *caller_data,
> - int iserr, const char *fmt, va_list al) {
> + bool iserr, const char *fmt, va_list al) {
> xc_interface *xch = caller_data;
>
> xc_reportv(xch,
> @@ -46,7 +46,7 @@ static void log_callback(struct elf_binary *elf, void
> *caller_data,
>
> void xc_elf_set_logfile(xc_interface *xch, struct elf_binary *elf,
> int verbose) {
> - elf_set_log(elf, log_callback, xch, verbose);
> + elf_set_log(elf, log_callback, xch, verbose /* convert to bool */);
Change the function prototype from int verbose to bool verbose ?
If not, for API reasons, then use !!verbose to correctly convert an
integer to a boolean.
~Andrew
> }
>
> /* ------------------------------------------------------------------------
> */
> @@ -82,7 +82,7 @@ static char *xc_dom_guest_type(struct xc_dom_image *dom,
> /* ------------------------------------------------------------------------
> */
> /* parse elf binary
> */
>
> -static int check_elf_kernel(struct xc_dom_image *dom, int verbose)
> +static int check_elf_kernel(struct xc_dom_image *dom, bool verbose)
> {
> if ( dom->kernel_blob == NULL )
> {
> @@ -110,7 +110,7 @@ static int xc_dom_probe_elf_kernel(struct xc_dom_image
> *dom)
> }
>
> static int xc_dom_load_elf_symtab(struct xc_dom_image *dom,
> - struct elf_binary *elf, int load)
> + struct elf_binary *elf, bool load)
> {
> struct elf_binary syms;
> ELF_HANDLE_DECL_NONCONST(elf_shdr) shdr; ELF_HANDLE_DECL(elf_shdr) shdr2;
> diff --git a/xen/common/libelf/libelf-dominfo.c
> b/xen/common/libelf/libelf-dominfo.c
> index b9a4e25..c4ced67 100644
> --- a/xen/common/libelf/libelf-dominfo.c
> +++ b/xen/common/libelf/libelf-dominfo.c
> @@ -101,7 +101,7 @@ int elf_xen_parse_note(struct elf_binary *elf,
> /* *INDENT-OFF* */
> static const struct {
> char *name;
> - int str;
> + bool str;
> } note_desc[] = {
> [XEN_ELFNOTE_ENTRY] = { "ENTRY", 0},
> [XEN_ELFNOTE_HYPERCALL_PAGE] = { "HYPERCALL_PAGE", 0},
> diff --git a/xen/common/libelf/libelf-loader.c
> b/xen/common/libelf/libelf-loader.c
> index 6c43c34..798f88b 100644
> --- a/xen/common/libelf/libelf-loader.c
> +++ b/xen/common/libelf/libelf-loader.c
> @@ -92,7 +92,7 @@ int elf_init(struct elf_binary *elf, const char
> *image_input, size_t size)
> }
>
> #ifndef __XEN__
> -void elf_call_log_callback(struct elf_binary *elf, int iserr,
> +void elf_call_log_callback(struct elf_binary *elf, bool iserr,
> const char *fmt,...) {
> va_list al;
>
> @@ -107,7 +107,7 @@ void elf_call_log_callback(struct elf_binary *elf, int
> iserr,
> }
>
> void elf_set_log(struct elf_binary *elf, elf_log_callback *log_callback,
> - void *log_caller_data, int verbose)
> + void *log_caller_data, bool verbose)
> {
> elf->log_callback = log_callback;
> elf->log_caller_data = log_caller_data;
> diff --git a/xen/common/libelf/libelf-private.h
> b/xen/common/libelf/libelf-private.h
> index 0bd9e66..ea7e197 100644
> --- a/xen/common/libelf/libelf-private.h
> +++ b/xen/common/libelf/libelf-private.h
> @@ -77,7 +77,7 @@
> #define elf_err(elf, fmt, args ... ) \
> elf_call_log_callback(elf, 1, fmt , ## args );
>
> -void elf_call_log_callback(struct elf_binary*, int iserr, const char
> *fmt,...);
> +void elf_call_log_callback(struct elf_binary*, bool iserr, const char
> *fmt,...);
>
> #define safe_strcpy(d,s) \
> do { strncpy((d),(s),sizeof((d))-1); \
> diff --git a/xen/common/libelf/libelf-tools.c
> b/xen/common/libelf/libelf-tools.c
> index b613593..0b7b2b6 100644
> --- a/xen/common/libelf/libelf-tools.c
> +++ b/xen/common/libelf/libelf-tools.c
> @@ -31,7 +31,7 @@ const char *elf_check_broken(const struct elf_binary *elf)
> return elf->broken;
> }
>
> -static int elf_ptrval_in_range(elf_ptrval ptrval, uint64_t size,
> +static bool elf_ptrval_in_range(elf_ptrval ptrval, uint64_t size,
> const void *region, uint64_t regionsize)
> /*
> * Returns true if the putative memory area [ptrval,ptrval+size>
> @@ -53,7 +53,7 @@ static int elf_ptrval_in_range(elf_ptrval ptrval, uint64_t
> size,
> return 1;
> }
>
> -int elf_access_ok(struct elf_binary * elf,
> +bool elf_access_ok(struct elf_binary * elf,
> uint64_t ptrval, size_t size)
> {
> if ( elf_ptrval_in_range(ptrval, size, elf->image_base, elf->size) )
> @@ -92,7 +92,7 @@ uint64_t elf_access_unsigned(struct elf_binary * elf,
> elf_ptrval base,
> uint64_t moreoffset, size_t size)
> {
> elf_ptrval ptrval = base + moreoffset;
> - int need_swap = elf_swap(elf);
> + bool need_swap = elf_swap(elf);
> const uint8_t *u8;
> const uint16_t *u16;
> const uint32_t *u32;
> @@ -332,7 +332,7 @@ ELF_HANDLE_DECL(elf_note) elf_note_next(struct elf_binary
> *elf, ELF_HANDLE_DECL(
>
> /* ------------------------------------------------------------------------
> */
>
> -int elf_is_elfbinary(const void *image_start, size_t image_size)
> +bool elf_is_elfbinary(const void *image_start, size_t image_size)
> {
> const Elf32_Ehdr *ehdr = image_start;
>
> @@ -342,7 +342,7 @@ int elf_is_elfbinary(const void *image_start, size_t
> image_size)
> return IS_ELF(*ehdr);
> }
>
> -int elf_phdr_is_loadable(struct elf_binary *elf, ELF_HANDLE_DECL(elf_phdr)
> phdr)
> +bool elf_phdr_is_loadable(struct elf_binary *elf, ELF_HANDLE_DECL(elf_phdr)
> phdr)
> {
> uint64_t p_type = elf_uval(elf, phdr, p_type);
> uint64_t p_flags = elf_uval(elf, phdr, p_flags);
> diff --git a/xen/include/xen/libelf.h b/xen/include/xen/libelf.h
> index c54c90b..1fa7421 100644
> --- a/xen/include/xen/libelf.h
> +++ b/xen/include/xen/libelf.h
> @@ -29,6 +29,8 @@
> #error define architectural endianness
> #endif
>
> +#include <stdbool.h>
> +
> #undef ELFSIZE
> #include "elfstructs.h"
> #ifdef __XEN__
> @@ -42,7 +44,7 @@
>
> struct elf_binary;
> typedef void elf_log_callback(struct elf_binary*, void *caller_data,
> - int iserr, const char *fmt, va_list al);
> + bool iserr, const char *fmt, va_list al);
>
> #endif
>
> @@ -236,7 +238,7 @@ struct elf_binary {
> elf_log_callback *log_callback;
> void *log_caller_data;
> #endif
> - int verbose;
> + bool verbose;
> const char *broken;
> };
>
> @@ -300,8 +302,8 @@ void elf_memset_safe(struct elf_binary*, elf_ptrval dst,
> int c, size_t);
> * outside permitted areas.
> */
>
> -int elf_access_ok(struct elf_binary * elf,
> - uint64_t ptrval, size_t size);
> +bool elf_access_ok(struct elf_binary * elf,
> + uint64_t ptrval, size_t size);
>
> #define elf_store_val(elf, type, ptr, val) \
> ({ \
> @@ -349,8 +351,8 @@ uint64_t elf_note_numeric_array(struct elf_binary *,
> ELF_HANDLE_DECL(elf_note),
> unsigned int unitsz, unsigned int idx);
> ELF_HANDLE_DECL(elf_note) elf_note_next(struct elf_binary *elf,
> ELF_HANDLE_DECL(elf_note) note);
>
> -int elf_is_elfbinary(const void *image_start, size_t image_size);
> -int elf_phdr_is_loadable(struct elf_binary *elf, ELF_HANDLE_DECL(elf_phdr)
> phdr);
> +bool elf_is_elfbinary(const void *image_start, size_t image_size);
> +bool elf_phdr_is_loadable(struct elf_binary *elf, ELF_HANDLE_DECL(elf_phdr)
> phdr);
>
> /* ------------------------------------------------------------------------
> */
> /* xc_libelf_loader.c
> */
> @@ -364,7 +366,7 @@ int elf_init(struct elf_binary *elf, const char *image,
> size_t size);
> void elf_set_verbose(struct elf_binary *elf);
> #else
> void elf_set_log(struct elf_binary *elf, elf_log_callback*,
> - void *log_caller_pointer, int verbose);
> + void *log_caller_pointer, bool verbose);
> #endif
>
> void elf_parse_binary(struct elf_binary *elf);
> @@ -416,7 +418,7 @@ struct elf_dom_parms {
> char xen_ver[16];
> char loader[16];
> int pae;
> - int bsd_symtab;
> + bool bsd_symtab;
> uint64_t virt_base;
> uint64_t virt_entry;
> uint64_t virt_hypercall;
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |