[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v2 1/2] libxc: Don't write terminating NULL character to command string



On 01/05/2016 05:42 PM, Andrew Cooper wrote:
On 05/01/2016 22:26, Boris Ostrovsky wrote:
When copying boot command string for HVMlite guests we explicitly write
'\0' at MAX_GUEST_CMDLINE offset. Unless the string is close to
MAX_GUEST_CMDLINE in length this write will end up in the wrong place,
beyond the end of the mapped range.

Instead we should test string's length early and error out if it is too
long.

Signed-off-by: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
MAX_GUEST_CMDLINE is an arbitrary and incorrect restriction.  It is
sadly baked into the PV ABI, but I specifically want to avoid lumbering
DMLite with the failings of PV.

By the looks of it, the only bug is the use of MAX_GUEST_CMDLINE.  The
xc_map_foreign_range() call already accounts for sufficient space to
store the string when mapping guest memory.

Yes, I was also thinking about dropping it but ended up keeping it mostly because it didn't feel right to blindly use strcpy().

-boris



I think you only need the 2nd hunk of this patch.

~Andrew

---
  tools/libxc/xc_dom_x86.c |    8 ++++++--
  1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/tools/libxc/xc_dom_x86.c b/tools/libxc/xc_dom_x86.c
index 3960875..b696149 100644
--- a/tools/libxc/xc_dom_x86.c
+++ b/tools/libxc/xc_dom_x86.c
@@ -647,6 +647,11 @@ static int alloc_magic_pages_hvm(struct xc_dom_image *dom)
          if ( dom->cmdline )
          {
              cmdline_size = ROUNDUP(strlen(dom->cmdline) + 1, 8);
+            if ( cmdline_size > MAX_GUEST_CMDLINE )
+            {
+                DOMPRINTF("Boot command line is too long");
+                goto error_out;
+            }
              start_info_size += cmdline_size;
}
@@ -676,8 +681,7 @@ static int alloc_magic_pages_hvm(struct xc_dom_image *dom)
if ( dom->cmdline )
          {
-            strncpy(cmdline, dom->cmdline, MAX_GUEST_CMDLINE);
-            cmdline[MAX_GUEST_CMDLINE - 1] = '\0';
+            strcpy(cmdline, dom->cmdline);
              start_info->cmdline_paddr = (seg.pfn << PAGE_SHIFT) +
                                  ((uintptr_t)cmdline - (uintptr_t)start_info);
          }


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.