[PATCH] libxc: x86: ensure that the initial mapping fits into the guest's memory

[PATCH] libxc: x86: ensure that the initial mapping fits into the guest's memory

[Ian Campbell]

On Fri, 2013-01-04 at 16:01 +0000, Xen.org security team wrote:
> 	     Xen Security Advisory CVE-2013-0154 / XSA-37
> 
>      Hypervisor crash due to incorrect ASSERT (debug build only)

This issue was exposed/triggered by a libxc bug, the fix for which is
below.

8<---------------------------------

# HG changeset patch
# User Ian Campbell <ijc@hellion.org.uk>
# Date 1357318325 0
# Node ID 84dea8458c36e6ee7d9347d32455cbeb5595825b
# Parent  7a61269a0c1ab33cc93b47f88d760f0d1f88eaab
libxc: x86: ensure that the initial mapping fits into the guest's memory

In particular we need to check that adding 512KB of slack and
rounding up to a 4MB boundary do not overflow the guest's memory
allocation. Otherwise we run off the end of the p2m when building the
guest's initial page tables and populate them with garbage.

Wei noticed this when build tiny (2MB) mini-os domains.

Reported-by: Wei Liu <Wei.Liu2@citrix.com>
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>

diff -r 7a61269a0c1a -r 84dea8458c36 tools/libxc/xc_dom_core.c
--- a/tools/libxc/xc_dom_core.c	Thu Jan 03 16:31:44 2013 +0000
+++ b/tools/libxc/xc_dom_core.c	Fri Jan 04 16:52:05 2013 +0000
@@ -873,7 +873,8 @@ int xc_dom_build_image(struct xc_dom_ima
         goto err;
     if ( dom->arch_hooks->count_pgtables )
     {
-        dom->arch_hooks->count_pgtables(dom);
+        if ( dom->arch_hooks->count_pgtables(dom) != 0 )
+            goto err;
         if ( (dom->pgtables > 0) &&
              (xc_dom_alloc_segment(dom, &dom->pgtables_seg, "page tables", 0,
                                    dom->pgtables * page_size) != 0) )
diff -r 7a61269a0c1a -r 84dea8458c36 tools/libxc/xc_dom_x86.c
--- a/tools/libxc/xc_dom_x86.c	Thu Jan 03 16:31:44 2013 +0000
+++ b/tools/libxc/xc_dom_x86.c	Fri Jan 04 16:52:05 2013 +0000
@@ -91,6 +91,15 @@ static int count_pgtables(struct xc_dom_
     {
         try_virt_end = round_up(dom->virt_alloc_end + pages * PAGE_SIZE_X86,
                                 bits_to_mask(22)); /* 4MB alignment */
+
+        if ( (try_virt_end >> PAGE_SHIFT_X86) > dom->total_pages )
+        {
+            xc_dom_panic(dom->xch, XC_OUT_OF_MEMORY,
+                         "%s: not enough memory for initial mapping",
+                         __FUNCTION__);
+            return -ENOMEM;
+        }
+
         dom->pg_l4 =
             nr_page_tables(dom, dom->parms.virt_base, try_virt_end, l4_bits);
         dom->pg_l3 =

Re: [PATCH] libxc: x86: ensure that the initial mapping fits into the guest's memory

[Jan Beulich]

>>> Ian Campbell <Ian.Campbell@citrix.com> 01/04/13 5:53 PM >>>
>libxc: x86: ensure that the initial mapping fits into the guest's memory
>
>In particular we need to check that adding 512KB of slack and
>rounding up to a 4MB boundary do not overflow the guest's memory
>allocation. Otherwise we run off the end of the p2m when building the
>guest's initial page tables and populate them with garbage.

Sadly our testing found this to cause SLE11 SP2 PV guests to not start
anymore (in its 4.1.x backported incarnation). I didn't get around yet to
check whether in the (apparently trivial) backport I overlooked something;
will do as soon as I get to the office.

Jan

Security support for debug=y builds (Was Re: Xen Security Advisory 37 (CVE-2013-0154) - Hypervisor crash due to incorrect ASSERT (debug build only))

[Ian Campbell]

On Fri, 2013-01-04 at 16:01 +0000, Xen.org security team wrote:
>      Hypervisor crash due to incorrect ASSERT (debug build only) 

While dealing with this issue the security team was faced with the
question as to whether bugs which are exposed only in debug=y builds
should be considered security relevant (i.e. would normally require an
embargo period, a full advisory, etc).

The Security Response Policy[0] does not offer any guidance on this
issue. We concluded that we should treat this issue as a normal Security
issue and then seek guidance from the community as to what we should do
in the future.

So what are your expectations for security sensitive bugs which only
affect debug builds? Note that debugging is disabled by default and that
we would recommended running non-debug builds in production.

Options which I can think of are:

      * debug=y bugs are Just Bugs and not security issues. i.e. they
        are discussed and fixed publicly on xen-devel and the fix is
        checked in in the usual way. There is no embargo or specific
        announcement. changelog may or may not refer to the security
        implications if debug=y is enabled.
      * debug=y bugs are security issues regardless, they are treated
        like any other security issue, i.e. following the process[0].
      * debug=y bugs are somewhere in the middle. (perhaps no embargo,
        less formal announcement etc etc)
      * ...

Any input appreciated. I will draft a process update as necessary based
on the response.

Ian.

[0] http://www.xen.org/projects/security_vulnerability_process.html

Re: [PATCH] libxc: x86: ensure that the initial mapping fits into the guest's memory

[Jan Beulich]

>>> On 07.01.13 at 08:00, Jan Beulich wrote:
>>>> Ian Campbell <Ian.Campbell@citrix.com> 01/04/13 5:53 PM >>>
>>libxc: x86: ensure that the initial mapping fits into the guest's memory
>>
>>In particular we need to check that adding 512KB of slack and
>>rounding up to a 4MB boundary do not overflow the guest's memory
>>allocation. Otherwise we run off the end of the p2m when building the
>>guest's initial page tables and populate them with garbage.
> 
> Sadly our testing found this to cause SLE11 SP2 PV guests to not start
> anymore (in its 4.1.x backported incarnation). I didn't get around yet to
> check whether in the (apparently trivial) backport I overlooked something;
> will do as soon as I get to the office.

Switching the added panic invocation to

            xc_dom_panic(dom->xch, XC_OUT_OF_MEMORY,
                         "%s: not enough memory for initial mapping (%#"PRIx64" > %#"PRIpfn")",
                         __FUNCTION__, try_virt_end >> PAGE_SHIFT_X86,
                         dom->total_pages);

I see (with xend on 4.1.3)

xc: error: panic: xc_dom_x86.c:100: count_pgtables: not enough memory for initial mapping (0xffffffff81bff > 0x80000): Out of memory

Did this really work for you? The 4.1.3 xl doesn't really want to work
for me, so I can't directly cross check whether there's a behavioral
difference between the two, but looking at an older log the virtual
addresses reported for virt_alloc_end look similar. Afaict you need
to subtract dom->parms.virt_base from try_virt_end.

Jan

Re: [PATCH] libxc: x86: ensure that the initial mapping fits into the guest's memory

[Ian Campbell]

On Mon, 2013-01-07 at 10:35 +0000, Jan Beulich wrote:
> >>> On 07.01.13 at 08:00, Jan Beulich wrote:
> >>>> Ian Campbell <Ian.Campbell@citrix.com> 01/04/13 5:53 PM >>>
> >>libxc: x86: ensure that the initial mapping fits into the guest's memory
> >>
> >>In particular we need to check that adding 512KB of slack and
> >>rounding up to a 4MB boundary do not overflow the guest's memory
> >>allocation. Otherwise we run off the end of the p2m when building the
> >>guest's initial page tables and populate them with garbage.
> > 
> > Sadly our testing found this to cause SLE11 SP2 PV guests to not start
> > anymore (in its 4.1.x backported incarnation). I didn't get around yet to
> > check whether in the (apparently trivial) backport I overlooked something;
> > will do as soon as I get to the office.
> 
> Switching the added panic invocation to
> 
>             xc_dom_panic(dom->xch, XC_OUT_OF_MEMORY,
>                          "%s: not enough memory for initial mapping (%#"PRIx64" > %#"PRIpfn")",
>                          __FUNCTION__, try_virt_end >> PAGE_SHIFT_X86,
>                          dom->total_pages);
> 
> I see (with xend on 4.1.3)
> 
> xc: error: panic: xc_dom_x86.c:100: count_pgtables: not enough memory for initial mapping (0xffffffff81bff > 0x80000): Out of memory
> 
> Did this really work for you?

It did but I must confess I only tested with the mini-os test domain,
since that was what the initial bug was reported about and I stupidly
didn't think to test with a "real" kernel.

>  The 4.1.3 xl doesn't really want to work
> for me, so I can't directly cross check whether there's a behavioral
> difference between the two, but looking at an older log the virtual
> addresses reported for virt_alloc_end look similar. Afaict you need
> to subtract dom->parms.virt_base from try_virt_end.

I bet virt_base == 0 for the mini-os kernel I tried. I'll respin and
retest.

Thanks and sorry for the cockup,

Ian.

Re: [PATCH] libxc: x86: ensure that the initial mapping fits into the guest's memory

[Jan Beulich]

>>> On 07.01.13 at 11:37, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Mon, 2013-01-07 at 10:35 +0000, Jan Beulich wrote:
>> >>> On 07.01.13 at 08:00, Jan Beulich wrote:
>> >>>> Ian Campbell <Ian.Campbell@citrix.com> 01/04/13 5:53 PM >>>
>> >>libxc: x86: ensure that the initial mapping fits into the guest's memory
>> >>
>> >>In particular we need to check that adding 512KB of slack and
>> >>rounding up to a 4MB boundary do not overflow the guest's memory
>> >>allocation. Otherwise we run off the end of the p2m when building the
>> >>guest's initial page tables and populate them with garbage.
>> > 
>> > Sadly our testing found this to cause SLE11 SP2 PV guests to not start
>> > anymore (in its 4.1.x backported incarnation). I didn't get around yet to
>> > check whether in the (apparently trivial) backport I overlooked something;
>> > will do as soon as I get to the office.
>> 
>> Switching the added panic invocation to
>> 
>>             xc_dom_panic(dom->xch, XC_OUT_OF_MEMORY,
>>                          "%s: not enough memory for initial mapping 
> (%#"PRIx64" > %#"PRIpfn")",
>>                          __FUNCTION__, try_virt_end >> PAGE_SHIFT_X86,
>>                          dom->total_pages);
>> 
>> I see (with xend on 4.1.3)
>> 
>> xc: error: panic: xc_dom_x86.c:100: count_pgtables: not enough memory for 
> initial mapping (0xffffffff81bff > 0x80000): Out of memory
>> 
>> Did this really work for you?
> 
> It did but I must confess I only tested with the mini-os test domain,
> since that was what the initial bug was reported about and I stupidly
> didn't think to test with a "real" kernel.
> 
>>  The 4.1.3 xl doesn't really want to work
>> for me, so I can't directly cross check whether there's a behavioral
>> difference between the two, but looking at an older log the virtual
>> addresses reported for virt_alloc_end look similar. Afaict you need
>> to subtract dom->parms.virt_base from try_virt_end.
> 
> I bet virt_base == 0 for the mini-os kernel I tried. I'll respin and
> retest.

This is what works for me (also added printing of the relevant value,
and dropping the unchanged parts of the patch):

--- a/tools/libxc/xc_dom_x86.c
+++ b/tools/libxc/xc_dom_x86.c
@@ -82,6 +82,7 @@ static int count_pgtables(struct xc_dom_
 {
     int pages, extra_pages;
     xen_vaddr_t try_virt_end;
+    xen_pfn_t try_pfn_end;
 
     extra_pages = dom->alloc_bootstack ? 1 : 0;
     extra_pages += dom->extra_pages;
@@ -91,6 +92,16 @@ static int count_pgtables(struct xc_dom_
     {
         try_virt_end = round_up(dom->virt_alloc_end + pages * PAGE_SIZE_X86,
                                 bits_to_mask(22)); /* 4MB alignment */
+        try_pfn_end = (try_virt_end - dom->parms.virt_base) >> PAGE_SHIFT_X86;
+
+        if ( try_pfn_end > dom->total_pages )
+        {
+            xc_dom_panic(dom->xch, XC_OUT_OF_MEMORY,
+                         "%s: not enough memory for initial mapping (%#"PRIpfn" > %#"PRIpfn")",
+                         __FUNCTION__, try_pfn_end, dom->total_pages);
+            return -ENOMEM;
+        }
+
         dom->pg_l4 =
             nr_page_tables(dom, dom->parms.virt_base, try_virt_end, l4_bits);
         dom->pg_l3 =

Jan

Re: Security support for debug=y builds (Was Re: Xen Security Advisory 37 (CVE-2013-0154) - Hypervisor crash due to incorrect ASSERT (debug build only))

[Keir Fraser]

On 07/01/2013 10:21, "Ian Campbell" <ijc@xen.org> wrote:

> On Fri, 2013-01-04 at 16:01 +0000, Xen.org security team wrote:
>>      Hypervisor crash due to incorrect ASSERT (debug build only)
> 
> While dealing with this issue the security team was faced with the
> question as to whether bugs which are exposed only in debug=y builds
> should be considered security relevant (i.e. would normally require an
> embargo period, a full advisory, etc).
> 
> The Security Response Policy[0] does not offer any guidance on this
> issue. We concluded that we should treat this issue as a normal Security
> issue and then seek guidance from the community as to what we should do
> in the future.
> 
> So what are your expectations for security sensitive bugs which only
> affect debug builds? Note that debugging is disabled by default and that
> we would recommended running non-debug builds in production.
> 
> Options which I can think of are:
> 
>       * debug=y bugs are Just Bugs and not security issues. i.e. they
>         are discussed and fixed publicly on xen-devel and the fix is
>         checked in in the usual way. There is no embargo or specific
>         announcement. changelog may or may not refer to the security
>         implications if debug=y is enabled.

This is my preference. I consider debug builds to be developer builds, and
wouldn't expect to see them used in production environments. We set debug=n
by default in our stable branches for that reason.

 -- Keir

>       * debug=y bugs are security issues regardless, they are treated
>         like any other security issue, i.e. following the process[0].
>       * debug=y bugs are somewhere in the middle. (perhaps no embargo,
>         less formal announcement etc etc)
>       * ...
> 
> Any input appreciated. I will draft a process update as necessary based
> on the response.
> 
> Ian.
> 
> [0] http://www.xen.org/projects/security_vulnerability_process.html
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Security support for debug=y builds (Was Re: Xen Security Advisory 37 (CVE-2013-0154) - Hypervisor crash due to incorrect ASSERT (debug build only))

[Jan Beulich]

>>> On 07.01.13 at 11:21, Ian Campbell <ijc@xen.org> wrote:
> Options which I can think of are:
> 
>       * debug=y bugs are Just Bugs and not security issues. i.e. they
>         are discussed and fixed publicly on xen-devel and the fix is
>         checked in in the usual way. There is no embargo or specific
>         announcement. changelog may or may not refer to the security
>         implications if debug=y is enabled.

+1

>       * debug=y bugs are security issues regardless, they are treated
>         like any other security issue, i.e. following the process[0].

-1

>       * debug=y bugs are somewhere in the middle. (perhaps no embargo,
>         less formal announcement etc etc)

+/-0

Jan

Re: Security support for debug=y builds (Was Re: Xen Security Advisory 37 (CVE-2013-0154) - Hypervisor crash due to incorrect ASSERT (debug build only))

[Andrew Cooper]

On 07/01/13 11:08, Keir Fraser wrote:
> On 07/01/2013 10:21, "Ian Campbell"<ijc@xen.org>  wrote:
>
>> On Fri, 2013-01-04 at 16:01 +0000, Xen.org security team wrote:
>>>       Hypervisor crash due to incorrect ASSERT (debug build only)
>> While dealing with this issue the security team was faced with the
>> question as to whether bugs which are exposed only in debug=y builds
>> should be considered security relevant (i.e. would normally require an
>> embargo period, a full advisory, etc).
>>
>> The Security Response Policy[0] does not offer any guidance on this
>> issue. We concluded that we should treat this issue as a normal Security
>> issue and then seek guidance from the community as to what we should do
>> in the future.
>>
>> So what are your expectations for security sensitive bugs which only
>> affect debug builds? Note that debugging is disabled by default and that
>> we would recommended running non-debug builds in production.
>>
>> Options which I can think of are:
>>
>>        * debug=y bugs are Just Bugs and not security issues. i.e. they
>>          are discussed and fixed publicly on xen-devel and the fix is
>>          checked in in the usual way. There is no embargo or specific
>>          announcement. changelog may or may not refer to the security
>>          implications if debug=y is enabled.
> This is my preference. I consider debug builds to be developer builds, and
> wouldn't expect to see them used in production environments. We set debug=n
> by default in our stable branches for that reason.
>
>   -- Keir

I second this opinion.  Production environments should not be running 
development builds.

~Andrew

>
>>        * debug=y bugs are security issues regardless, they are treated
>>          like any other security issue, i.e. following the process[0].
>>        * debug=y bugs are somewhere in the middle. (perhaps no embargo,
>>          less formal announcement etc etc)
>>        * ...
>>
>> Any input appreciated. I will draft a process update as necessary based
>> on the response.
>>
>> Ian.
>>
>> [0] http://www.xen.org/projects/security_vulnerability_process.html
>>
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xen.org
>> http://lists.xen.org/xen-devel
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: Security support for debug=y builds (Was Re: Xen Security Advisory 37 (CVE-2013-0154) - Hypervisor crash due to incorrect ASSERT (debug build only))

[Ian Campbell]

On Mon, 2013-01-07 at 11:21 +0000, Andrew Cooper wrote:
> On 07/01/13 11:08, Keir Fraser wrote:
> > On 07/01/2013 10:21, "Ian Campbell"<ijc@xen.org>  wrote:
> >>        * debug=y bugs are Just Bugs and not security issues. i.e. they
> >>          are discussed and fixed publicly on xen-devel and the fix is
> >>          checked in in the usual way. There is no embargo or specific
> >>          announcement. changelog may or may not refer to the security
> >>          implications if debug=y is enabled.
> > This is my preference. I consider debug builds to be developer builds, and
> > wouldn't expect to see them used in production environments. We set debug=n
> > by default in our stable branches for that reason.
> >
> >   -- Keir
> 
> I second this opinion.  Production environments should not be running 
> development builds.

I tried to keep my initial mail unbiased, but this is my opinion too.

Ian.

Re: [PATCH] libxc: x86: ensure that the initial mapping fits into the guest's memory

[Ian Campbell]

On Mon, 2013-01-07 at 11:05 +0000, Jan Beulich wrote:
> >>  The 4.1.3 xl doesn't really want to work
> >> for me, so I can't directly cross check whether there's a behavioral
> >> difference between the two, but looking at an older log the virtual
> >> addresses reported for virt_alloc_end look similar. Afaict you need
> >> to subtract dom->parms.virt_base from try_virt_end.
> > 
> > I bet virt_base == 0 for the mini-os kernel I tried. I'll respin and
> > retest.
> 
> This is what works for me (also added printing of the relevant value,
> and dropping the unchanged parts of the patch):

This looks sensible to me, thanks. I shall respin the complete patch
shortly.

Ian.

> 
> --- a/tools/libxc/xc_dom_x86.c
> +++ b/tools/libxc/xc_dom_x86.c
> @@ -82,6 +82,7 @@ static int count_pgtables(struct xc_dom_
>  {
>      int pages, extra_pages;
>      xen_vaddr_t try_virt_end;
> +    xen_pfn_t try_pfn_end;
>  
>      extra_pages = dom->alloc_bootstack ? 1 : 0;
>      extra_pages += dom->extra_pages;
> @@ -91,6 +92,16 @@ static int count_pgtables(struct xc_dom_
>      {
>          try_virt_end = round_up(dom->virt_alloc_end + pages * PAGE_SIZE_X86,
>                                  bits_to_mask(22)); /* 4MB alignment */
> +        try_pfn_end = (try_virt_end - dom->parms.virt_base) >> PAGE_SHIFT_X86;
> +
> +        if ( try_pfn_end > dom->total_pages )
> +        {
> +            xc_dom_panic(dom->xch, XC_OUT_OF_MEMORY,
> +                         "%s: not enough memory for initial mapping (%#"PRIpfn" > %#"PRIpfn")",
> +                         __FUNCTION__, try_pfn_end, dom->total_pages);
> +            return -ENOMEM;
> +        }
> +
>          dom->pg_l4 =
>              nr_page_tables(dom, dom->parms.virt_base, try_virt_end, l4_bits);
>          dom->pg_l3 =
> 
> Jan
> 

Re: Security support for debug=y builds (Was Re: Xen Security Advisory 37 (CVE-2013-0154) - Hypervisor crash due to incorrect ASSERT (debug build only))

[James Bulpin]

On Mon, 2013-01-07 at 11:21 +0000, Andrew Cooper wrote:
> On 07/01/13 11:08, Keir Fraser wrote:
> > On 07/01/2013 10:21, "Ian Campbell"<ijc@xen.org>  wrote:
> >>        * debug=y bugs are Just Bugs and not security issues. i.e. they
> >>          are discussed and fixed publicly on xen-devel and the fix is
> >>          checked in in the usual way. There is no embargo or specific
> >>          announcement. changelog may or may not refer to the security
> >>          implications if debug=y is enabled.
> > This is my preference. I consider debug builds to be developer builds, and
> > wouldn't expect to see them used in production environments. We set debug=n
> > by default in our stable branches for that reason.
> >
> >   -- Keir
>
> I second this opinion.  Production environments should not be running
> development builds.

+1 but I'd still like to see such issues backported to stable branches.

Cheers,
James

[PATCH V2] libxc: x86: ensure that the initial mapping fits into the guest's memory

[Ian Campbell]

On Mon, 2013-01-07 at 11:39 +0000, Ian Campbell wrote:
> I shall respin the complete patch shortly.
8<----------------------------------

# HG changeset patch
# User Ian Campbell <ijc@hellion.org.uk>
# Date 1357565755 0
# Node ID c95c8ea2424b928be44550f29119f5e58f628f8b
# Parent  7a61269a0c1ab33cc93b47f88d760f0d1f88eaab
libxc: x86: ensure that the initial mapping fits into the guest's memory

In particular we need to check that adding 512KB of slack and
rounding up to a 4MB boundary do not overflow the guest's memory
allocation. Otherwise we run off the end of the p2m when building the
guest's initial page tables and populate them with garbage.

Wei noticed this when build tiny (2MB) mini-os domains.

Reported-by: Wei Liu <Wei.Liu2@citrix.com>
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
v2: Correctly account for virt_base, thanks Jan. Now tested with 32 and 
    64 bit pvops kernels.

diff -r 7a61269a0c1a -r c95c8ea2424b tools/libxc/xc_dom_core.c
--- a/tools/libxc/xc_dom_core.c	Thu Jan 03 16:31:44 2013 +0000
+++ b/tools/libxc/xc_dom_core.c	Mon Jan 07 13:35:55 2013 +0000
@@ -873,7 +873,8 @@ int xc_dom_build_image(struct xc_dom_ima
         goto err;
     if ( dom->arch_hooks->count_pgtables )
     {
-        dom->arch_hooks->count_pgtables(dom);
+        if ( dom->arch_hooks->count_pgtables(dom) != 0 )
+            goto err;
         if ( (dom->pgtables > 0) &&
              (xc_dom_alloc_segment(dom, &dom->pgtables_seg, "page tables", 0,
                                    dom->pgtables * page_size) != 0) )
diff -r 7a61269a0c1a -r c95c8ea2424b tools/libxc/xc_dom_x86.c
--- a/tools/libxc/xc_dom_x86.c	Thu Jan 03 16:31:44 2013 +0000
+++ b/tools/libxc/xc_dom_x86.c	Mon Jan 07 13:35:55 2013 +0000
@@ -82,6 +82,7 @@ static int count_pgtables(struct xc_dom_
 {
     int pages, extra_pages;
     xen_vaddr_t try_virt_end;
+    xen_pfn_t try_pfn_end;
 
     extra_pages = dom->alloc_bootstack ? 1 : 0;
     extra_pages += dom->extra_pages;
@@ -91,6 +92,17 @@ static int count_pgtables(struct xc_dom_
     {
         try_virt_end = round_up(dom->virt_alloc_end + pages * PAGE_SIZE_X86,
                                 bits_to_mask(22)); /* 4MB alignment */
+
+        try_pfn_end = (try_virt_end - dom->parms.virt_base) >> PAGE_SHIFT_X86;
+
+        if ( try_pfn_end > dom->total_pages )
+        {
+            xc_dom_panic(dom->xch, XC_OUT_OF_MEMORY,
+                         "%s: not enough memory for initial mapping (%#"PRIpfn" > %#"PRIpfn")",
+                         __FUNCTION__, try_pfn_end, dom->total_pages);
+            return -ENOMEM;
+        }
+
         dom->pg_l4 =
             nr_page_tables(dom, dom->parms.virt_base, try_virt_end, l4_bits);
         dom->pg_l3 =

Re: Security support for debug=y builds (Was Re: Xen Security Advisory 37 (CVE-2013-0154) - Hypervisor crash due to incorrect ASSERT (debug build only))

[Keir Fraser]

On 07/01/2013 12:58, "James Bulpin" <James.Bulpin@eu.citrix.com> wrote:

> On Mon, 2013-01-07 at 11:21 +0000, Andrew Cooper wrote:
>> On 07/01/13 11:08, Keir Fraser wrote:
>>> On 07/01/2013 10:21, "Ian Campbell"<ijc@xen.org>  wrote:
>>>>        * debug=y bugs are Just Bugs and not security issues. i.e. they
>>>>          are discussed and fixed publicly on xen-devel and the fix is
>>>>          checked in in the usual way. There is no embargo or specific
>>>>          announcement. changelog may or may not refer to the security
>>>>          implications if debug=y is enabled.
>>> This is my preference. I consider debug builds to be developer builds, and
>>> wouldn't expect to see them used in production environments. We set debug=n
>>> by default in our stable branches for that reason.
>>> 
>>>   -- Keir
>> 
>> I second this opinion.  Production environments should not be running
>> development builds.
> 
> +1 but I'd still like to see such issues backported to stable branches.

Yes, this already happens and will not change.

 -- Keir

> Cheers,
> James
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: Security support for debug=y builds (Was Re: Xen Security Advisory 37 (CVE-2013-0154) - Hypervisor crash due to incorrect ASSERT (debug build only))

[Ian Campbell]

On Mon, 2013-01-07 at 11:36 +0000, Ian Campbell wrote:
> On Mon, 2013-01-07 at 11:21 +0000, Andrew Cooper wrote:
> > On 07/01/13 11:08, Keir Fraser wrote:
> > > On 07/01/2013 10:21, "Ian Campbell"<ijc@xen.org>  wrote:
> > >>        * debug=y bugs are Just Bugs and not security issues. i.e. they
> > >>          are discussed and fixed publicly on xen-devel and the fix is
> > >>          checked in in the usual way. There is no embargo or specific
> > >>          announcement. changelog may or may not refer to the security
> > >>          implications if debug=y is enabled.
> > > This is my preference. I consider debug builds to be developer builds, and
> > > wouldn't expect to see them used in production environments. We set debug=n
> > > by default in our stable branches for that reason.
> > >
> > >   -- Keir
> > 
> > I second this opinion.  Production environments should not be running 
> > development builds.
> 
> I tried to keep my initial mail unbiased, but this is my opinion too.

Looks like we have a consensus on this then.

Lars, could you add some words to the doc?

e.g. under "Scope of this process".

This process primarily covers the Xen Hypervisor Project. Vulnerabilties
reported against other Xen.org projects will be handled on a best effort
basis by the relevant Project Lead together with the security team.

Re: Security support for debug=y builds (Was Re: Xen Security Advisory 37 (CVE-2013-0154) - Hypervisor crash due to incorrect ASSERT (debug build only))

[Ian Campbell]

On Mon, 2013-01-07 at 11:36 +0000, Ian Campbell wrote:
> On Mon, 2013-01-07 at 11:21 +0000, Andrew Cooper wrote:
> > On 07/01/13 11:08, Keir Fraser wrote:
> > > On 07/01/2013 10:21, "Ian Campbell"<ijc@xen.org>  wrote:
> > >>        * debug=y bugs are Just Bugs and not security issues. i.e. they
> > >>          are discussed and fixed publicly on xen-devel and the fix is
> > >>          checked in in the usual way. There is no embargo or specific
> > >>          announcement. changelog may or may not refer to the security
> > >>          implications if debug=y is enabled.
> > > This is my preference. I consider debug builds to be developer builds, and
> > > wouldn't expect to see them used in production environments. We set debug=n
> > > by default in our stable branches for that reason.
> > >
> > >   -- Keir
> > 
> > I second this opinion.  Production environments should not be running 
> > development builds.
> 
> I tried to keep my initial mail unbiased, but this is my opinion too.

Looks like we have a consensus on this then.

I'm in two minds about whether this needs to be made explicit in the
vulnerability process.

If it were then e.g. "Scope of this process" could become:

        This process primarily covers the Xen Hypervisor Project and
        covers production configurations only, that is builds without
        debugging features enabled, e.g. debug=y.
        
        Vulnerabilties reported against other Xen.org projects will be
        handled on a best effort basis by the relevant Project Lead
        together with the security team.
        
Ian.

Re: Security support for debug=y builds (Was Re: Xen Security Advisory 37 (CVE-2013-0154) - Hypervisor crash due to incorrect ASSERT (debug build only))

[Ian Campbell]

Ignore this one, hit send too soon.

Re: Security support for debug=y builds (Was Re: Xen Security Advisory 37 (CVE-2013-0154) - Hypervisor crash due to incorrect ASSERT (debug build only))

[Jan Beulich]

>>> On 08.02.13 at 12:29, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> If it were then e.g. "Scope of this process" could become:
> 
>         This process primarily covers the Xen Hypervisor Project and
>         covers production configurations only, that is builds without
>         debugging features enabled, e.g. debug=y.

To me, with the wording above, the "debug=y" example here is
ambiguous (in that I could read it to mean only "debug=y" is
covered by the process, even if I agree that this makes little
sense).

Jan

>         Vulnerabilties reported against other Xen.org projects will be
>         handled on a best effort basis by the relevant Project Lead
>         together with the security team.
>         
> Ian.

Re: Security support for debug=y builds (Was Re: Xen Security Advisory 37 (CVE-2013-0154) - Hypervisor crash due to incorrect ASSERT (debug build only))

[Ian Campbell]

On Fri, 2013-02-08 at 11:40 +0000, Jan Beulich wrote:
> >>> On 08.02.13 at 12:29, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> > If it were then e.g. "Scope of this process" could become:
> > 
> >         This process primarily covers the Xen Hypervisor Project and
> >         covers production configurations only, that is builds without
> >         debugging features enabled, e.g. debug=y.
> 
> To me, with the wording above, the "debug=y" example here is
> ambiguous (in that I could read it to mean only "debug=y" is
> covered by the process, even if I agree that this makes little
> sense).

yes, you are right.

I was trying to avoid having to explicitly list all the options while
also avoiding suggesting that other debug options, like perfc=y or (in
the future) coverage=y, might be supported.

Perhaps
        .... Therefore configurations built with e.g. debug=y are not
        covered by this process.

Ian.