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 03/22] libxc: Fix range checking in xc_dom_pfn_to_ptr etc.
Date: Tue, 11 Jun 2013 19:20:46 +0100	[thread overview]
Message-ID: <1370974865-19554-4-git-send-email-ian.jackson@eu.citrix.com> (raw)
In-Reply-To: <1370974865-19554-1-git-send-email-ian.jackson@eu.citrix.com>

* Ensure that xc_dom_pfn_to_ptr (when called with count==0) does not
  return a previously-allocated block which is entirely before the
  requested pfn (!)

* Provide a version of xc_dom_pfn_to_ptr, xc_dom_pfn_to_ptr_retcount,
  which provides the length of the mapped region via an out parameter.

* Change xc_dom_vaddr_to_ptr to always provide the length of the
  mapped region and change the call site in xc_dom_binloader.c to
  check it.  The call site in xc_dom_load_elf_symtab will be corrected
  in a forthcoming patch, and for now ignores the returned length.

This is part of the fix to a security issue, XSA-55.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>

v5: This patch is new in v5 of the series.
---
 tools/libxc/xc_dom.h           |   16 +++++++++++++---
 tools/libxc/xc_dom_binloader.c |   11 ++++++++++-
 tools/libxc/xc_dom_core.c      |   13 +++++++++++++
 tools/libxc/xc_dom_elfloader.c |    3 ++-
 4 files changed, 38 insertions(+), 5 deletions(-)

diff --git a/tools/libxc/xc_dom.h b/tools/libxc/xc_dom.h
index 316c5cb..ad6fdd4 100644
--- a/tools/libxc/xc_dom.h
+++ b/tools/libxc/xc_dom.h
@@ -291,6 +291,8 @@ int xc_dom_alloc_segment(struct xc_dom_image *dom,
 
 void *xc_dom_pfn_to_ptr(struct xc_dom_image *dom, xen_pfn_t first,
                         xen_pfn_t count);
+void *xc_dom_pfn_to_ptr_retcount(struct xc_dom_image *dom, xen_pfn_t first,
+                                 xen_pfn_t count, xen_pfn_t *count_out);
 void xc_dom_unmap_one(struct xc_dom_image *dom, xen_pfn_t pfn);
 void xc_dom_unmap_all(struct xc_dom_image *dom);
 
@@ -318,13 +320,21 @@ static inline void *xc_dom_seg_to_ptr(struct xc_dom_image *dom,
 }
 
 static inline void *xc_dom_vaddr_to_ptr(struct xc_dom_image *dom,
-                                        xen_vaddr_t vaddr)
+                                        xen_vaddr_t vaddr,
+                                        size_t *safe_region_out)
 {
     unsigned int page_size = XC_DOM_PAGE_SIZE(dom);
     xen_pfn_t page = (vaddr - dom->parms.virt_base) / page_size;
     unsigned int offset = (vaddr - dom->parms.virt_base) % page_size;
-    void *ptr = xc_dom_pfn_to_ptr(dom, page, 0);
-    return (ptr ? (ptr + offset) : NULL);
+    xen_pfn_t safe_region_count;
+    void *ptr;
+
+    *safe_region_out = 0;
+    ptr = xc_dom_pfn_to_ptr_retcount(dom, page, 0, &safe_region_count);
+    if ( ptr == NULL )
+        return ptr;
+    *safe_region_out = (safe_region_count << XC_DOM_PAGE_SHIFT(dom)) - offset;
+    return ptr;
 }
 
 static inline xen_pfn_t xc_dom_p2m_host(struct xc_dom_image *dom, xen_pfn_t pfn)
diff --git a/tools/libxc/xc_dom_binloader.c b/tools/libxc/xc_dom_binloader.c
index c14727c..d2de04c 100644
--- a/tools/libxc/xc_dom_binloader.c
+++ b/tools/libxc/xc_dom_binloader.c
@@ -249,6 +249,7 @@ static int xc_dom_load_bin_kernel(struct xc_dom_image *dom)
     char *image = dom->kernel_blob;
     char *dest;
     size_t image_size = dom->kernel_size;
+    size_t dest_size;
     uint32_t start_addr;
     uint32_t load_end_addr;
     uint32_t bss_end_addr;
@@ -272,7 +273,15 @@ static int xc_dom_load_bin_kernel(struct xc_dom_image *dom)
     DOMPRINTF("  text_size: 0x%" PRIx32 "", text_size);
     DOMPRINTF("  bss_size:  0x%" PRIx32 "", bss_size);
 
-    dest = xc_dom_vaddr_to_ptr(dom, dom->kernel_seg.vstart);
+    dest = xc_dom_vaddr_to_ptr(dom, dom->kernel_seg.vstart, &dest_size);
+
+    if ( dest_size < text_size ||
+         dest_size - text_size < bss_size )
+    {
+        DOMPRINTF("%s: mapped region is too small for image", __FUNCTION__);
+        return -EINVAL;
+    }
+
     memcpy(dest, image + skip, text_size);
     memset(dest + text_size, 0, bss_size);
 
diff --git a/tools/libxc/xc_dom_core.c b/tools/libxc/xc_dom_core.c
index b92e4a9..cf96bfa 100644
--- a/tools/libxc/xc_dom_core.c
+++ b/tools/libxc/xc_dom_core.c
@@ -351,11 +351,20 @@ int xc_dom_try_gunzip(struct xc_dom_image *dom, void **blob, size_t * size)
 void *xc_dom_pfn_to_ptr(struct xc_dom_image *dom, xen_pfn_t pfn,
                         xen_pfn_t count)
 {
+    xen_pfn_t count_out_dummy;
+    return xc_dom_pfn_to_ptr_retcount(dom, pfn, count, &count_out_dummy);
+}
+
+void *xc_dom_pfn_to_ptr_retcount(struct xc_dom_image *dom, xen_pfn_t pfn,
+                                 xen_pfn_t count, xen_pfn_t *count_out)
+{
     struct xc_dom_phys *phys;
     xen_pfn_t offset;
     unsigned int page_shift = XC_DOM_PAGE_SHIFT(dom);
     char *mode = "unset";
 
+    *count_out = 0;
+
     offset = pfn - dom->rambase_pfn;
     if ( offset > dom->total_pages || /* multiple checks to avoid overflows */
          count > dom->total_pages ||
@@ -386,6 +395,7 @@ void *xc_dom_pfn_to_ptr(struct xc_dom_image *dom, xen_pfn_t pfn,
                           phys->count);
                 return NULL;
             }
+            *count_out = count;
         }
         else
         {
@@ -393,6 +403,9 @@ void *xc_dom_pfn_to_ptr(struct xc_dom_image *dom, xen_pfn_t pfn,
                just hand out a pointer to it */
             if ( pfn < phys->first )
                 continue;
+            if ( pfn >= phys->first + phys->count )
+                continue;
+            *count_out = phys->count - (pfn - phys->first);
         }
         return phys->ptr + ((pfn - phys->first) << page_shift);
     }
diff --git a/tools/libxc/xc_dom_elfloader.c b/tools/libxc/xc_dom_elfloader.c
index 6583859..bc92302 100644
--- a/tools/libxc/xc_dom_elfloader.c
+++ b/tools/libxc/xc_dom_elfloader.c
@@ -128,10 +128,11 @@ static int xc_dom_load_elf_symtab(struct xc_dom_image *dom,
 
     if ( load )
     {
+        size_t allow_size; /* will be used in a forthcoming XSA-55 patch */
         if ( !dom->bsd_symtab_start )
             return 0;
         size = dom->kernel_seg.vend - dom->bsd_symtab_start;
-        hdr  = xc_dom_vaddr_to_ptr(dom, dom->bsd_symtab_start);
+        hdr  = xc_dom_vaddr_to_ptr(dom, dom->bsd_symtab_start, &allow_size);
         *(int *)hdr = size - sizeof(int);
     }
     else
-- 
1.7.2.5

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

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