| Message ID | 1452110602-3570-2-git-send-email-boris.ostrovsky@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jan 06, 2016 at 03:03:21PM -0500, 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. > > We don't need to limit the size of command string to some arbitrary > number. Any size that can be successfully allocated and mapped is valid > and so the string is guaranteed to be NULL-terminated (since we use > strlen, which needs terminating '\0', to calculate allocation size). > > Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com> Acked-by: Wei Liu <wei.liu2@citrix.com> > --- > tools/libxc/xc_dom_x86.c | 3 +-- > 1 files changed, 1 insertions(+), 2 deletions(-) > > diff --git a/tools/libxc/xc_dom_x86.c b/tools/libxc/xc_dom_x86.c > index 3960875..b8d2904 100644 > --- a/tools/libxc/xc_dom_x86.c > +++ b/tools/libxc/xc_dom_x86.c > @@ -676,8 +676,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'; > + strncpy(cmdline, dom->cmdline, cmdline_size); > start_info->cmdline_paddr = (seg.pfn << PAGE_SHIFT) + > ((uintptr_t)cmdline - (uintptr_t)start_info); > } > -- > 1.7.1 >
On Thu, 2016-01-07 at 11:19 +0000, Wei Liu wrote: > On Wed, Jan 06, 2016 at 03:03:21PM -0500, 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. > > > > We don't need to limit the size of command string to some arbitrary > > number. Any size that can be successfully allocated and mapped is valid > > and so the string is guaranteed to be NULL-terminated (since we use > > strlen, which needs terminating '\0', to calculate allocation size). > > > > Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com> > > Acked-by: Wei Liu <wei.liu2@citrix.com> Applied. Roger commented on #2 so I didn't take that, but this seemed to standalone.
diff --git a/tools/libxc/xc_dom_x86.c b/tools/libxc/xc_dom_x86.c index 3960875..b8d2904 100644 --- a/tools/libxc/xc_dom_x86.c +++ b/tools/libxc/xc_dom_x86.c @@ -676,8 +676,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'; + strncpy(cmdline, dom->cmdline, cmdline_size); start_info->cmdline_paddr = (seg.pfn << PAGE_SHIFT) + ((uintptr_t)cmdline - (uintptr_t)start_info); }
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. We don't need to limit the size of command string to some arbitrary number. Any size that can be successfully allocated and mapped is valid and so the string is guaranteed to be NULL-terminated (since we use strlen, which needs terminating '\0', to calculate allocation size). Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com> --- tools/libxc/xc_dom_x86.c | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-)