All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ian Jackson <ian.jackson@eu.citrix.com>
To: xen-devel@lists.xensource.com
Cc: andrew.cooper3@citrix.com, mattjd@gmail.com,
	Ian Jackson <ian.jackson@eu.citrix.com>,
	security@xen.org
Subject: [PATCH 14/22] libelf: use C99 bool for booleans
Date: Tue, 11 Jun 2013 19:22:32 +0100	[thread overview]
Message-ID: <1370974960-19824-15-git-send-email-ian.jackson@eu.citrix.com> (raw)
In-Reply-To: <1370974960-19824-1-git-send-email-ian.jackson@eu.citrix.com>

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@eu.citrix.com>
Acked-by: George Dunlap <george.dunlap@eu.citrix.com>
---
 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 4fb4da2..9ba64ae 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 */);
 }
 
 /* ------------------------------------------------------------------------ */
@@ -84,7 +84,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 )
     {
@@ -112,7 +112,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 98c80dc..12b6c2a 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 f8be635..0dccd4d 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 280dfd1..277be04 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 744027e..fa58f76 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 ac93858..951430f 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
 
@@ -237,7 +239,7 @@ struct elf_binary {
     elf_log_callback *log_callback;
     void *log_caller_data;
 #endif
-    int verbose;
+    bool verbose;
     const char *broken;
 };
 
@@ -301,8 +303,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)                              \
     ({                                                                  \
@@ -351,9 +353,9 @@ uint64_t elf_note_numeric_array(struct elf_binary *, ELF_HANDLE_DECL(elf_note),
 ELF_HANDLE_DECL(elf_note) elf_note_next(struct elf_binary *elf, ELF_HANDLE_DECL(elf_note) note);
 
 /* (Only) checks that the image has the right magic number. */
-int elf_is_elfbinary(const void *image_start, size_t image_size);
+bool 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_phdr_is_loadable(struct elf_binary *elf, ELF_HANDLE_DECL(elf_phdr) phdr);
 
 /* ------------------------------------------------------------------------ */
 /* xc_libelf_loader.c                                                       */
@@ -367,7 +369,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);
@@ -419,7 +421,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;
-- 
1.7.2.5

  parent reply	other threads:[~2013-06-11 18:22 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-06-11 18:22 [PATCH v7 00/22] XSA55 libelf fixes for Xen 4.2 Ian Jackson
2013-06-11 18:22 ` [PATCH 01/22] libelf: abolish libelf-relocate.c Ian Jackson
2013-06-11 18:22 ` [PATCH 02/22] libxc: introduce xc_dom_seg_to_ptr_pages Ian Jackson
2013-06-11 18:22 ` [PATCH 03/22] libxc: Fix range checking in xc_dom_pfn_to_ptr etc Ian Jackson
2013-06-11 18:22 ` [PATCH 04/22] libelf: add `struct elf_binary*' parameter to elf_load_image Ian Jackson
2013-06-11 18:22 ` [PATCH 05/22] libelf: abolish elf_sval and elf_access_signed Ian Jackson
2013-06-11 18:22 ` [PATCH 06/22] libelf: move include of <asm/guest_access.h> to top of file Ian Jackson
2013-06-11 18:22 ` [PATCH 07/22] libelf/xc_dom_load_elf_symtab: Do not use "syms" uninitialised Ian Jackson
2013-06-11 18:22 ` [PATCH 08/22] libelf: introduce macros for memory access and pointer handling Ian Jackson
2013-06-11 18:22 ` [PATCH 09/22] tools/xcutils/readnotes: adjust print_l1_mfn_valid_note Ian Jackson
2013-06-11 18:22 ` [PATCH 10/22] libelf: check nul-terminated strings properly Ian Jackson
2013-06-11 18:22 ` [PATCH 11/22] libelf: check all pointer accesses Ian Jackson
2013-06-11 18:22 ` [PATCH 12/22] libelf: Check pointer references in elf_is_elfbinary Ian Jackson
2013-06-11 18:22 ` [PATCH 13/22] libelf: Make all callers call elf_check_broken Ian Jackson
2013-06-11 18:22 ` Ian Jackson [this message]
2013-06-11 18:22 ` [PATCH 15/22] libelf: use only unsigned integers Ian Jackson
2013-06-11 18:22 ` [PATCH 16/22] libelf: check loops for running away Ian Jackson
2013-06-11 18:22 ` [PATCH 17/22] libelf: abolish obsolete macros Ian Jackson
2013-06-11 18:22 ` [PATCH 18/22] libxc: Add range checking to xc_dom_binloader Ian Jackson
2013-06-11 18:22 ` [PATCH 19/22] libxc: check failure of xc_dom_*_to_ptr, xc_map_foreign_range Ian Jackson
2013-06-13 15:22   ` Ian Jackson
2013-06-13 15:41     ` Andrew Cooper
2013-06-11 18:22 ` [PATCH 20/22] libxc: check return values from malloc Ian Jackson
2013-06-11 18:22 ` [PATCH 21/22] libxc: range checks in xc_dom_p2m_host and _guest Ian Jackson
2013-06-11 18:22 ` [PATCH 22/22] libxc: check blob size before proceeding in xc_dom_check_gzip Ian Jackson
  -- strict thread matches above, loose matches on Subject: below --
2013-06-11 18:20 [PATCH v7 00/22] XSA55 libelf fixes for unstable Ian Jackson
2013-06-11 18:20 ` [PATCH 14/22] libelf: use C99 bool for booleans Ian Jackson
2013-06-11 19:04   ` Andrew Cooper
2013-06-07 18:35 [PATCH v6 00/22] XSA55 libelf fixes for Xen 4.2 Ian Jackson
2013-06-07 18:35 ` [PATCH 14/22] libelf: use C99 bool for booleans Ian Jackson
2013-06-07 18:27 [PATCH v6 00/22] XSA55 libelf fixes for unstable Ian Jackson
2013-06-07 18:27 ` [PATCH 14/22] libelf: use C99 bool for booleans Ian Jackson
2013-06-10 22:09   ` Andrew Cooper
2013-06-11  7:17     ` Jan Beulich
2013-06-11 14:50     ` Ian Jackson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1370974960-19824-15-git-send-email-ian.jackson@eu.citrix.com \
    --to=ian.jackson@eu.citrix.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=mattjd@gmail.com \
    --cc=security@xen.org \
    --cc=xen-devel@lists.xensource.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.